fix: make conditional editing work with lazy columns#11405
Conversation
There was a problem hiding this comment.
There's an error that happens in Firefox and Safari when you scroll away from the column while the editor is active, so that the column is detached, and then scroll back to the same column and try to open the editor on the same cell again. That seems to happen because _stopEdit is never called when the element is detached whereas in Chrome it does:
grid-edit-firefox.mp4
There was a problem hiding this comment.
Looks like those browsers don't fire focusout when the focused element is removed from the DOM. Added an override for __updateColumnsBodyContentHidden to detect when the edited cell is removed from the DOM.
The added tests only covers Safari, as Playwright Firefox does fire focusout in that case 🤦♂️
There was a problem hiding this comment.
Yeah, I recall facing this same limitation in another issue (maybe also related to grid-pro 🤔). I can't see the issue anymore.
| }); | ||
| }); | ||
|
|
||
| describe('lazy column rendering', () => { |
There was a problem hiding this comment.
In line 542 of the vaadin-grid-pro-inline-editing-mixin file, it checks that cell.parentNode === row. It should use cell.__parentRow === row for consistency with the rest of the PR. Without this, item identity changes are not detected when the edited cell is lazy-detached.
This test fails without the suggested change.
it('should stop editing when item identity changes for a detached edited cell', async () => {
let cell = getCellByColumnPath('col1');
cell.focus();
await sendKeys({ press: 'Enter' });
expect(getCellEditor(cell)).to.be.ok;
// Scroll far right so col1 cell gets detached from the DOM
await scrollHorizontally(400);
// Change the item identity for the edited row
grid.items = [
{ col0: 'x0', col1: 'x1', col2: 'x2', col3: 'x3', col4: 'x4', col5: 'x5', col6: 'x6', col7: 'x7' },
{ col0: 'b0', col1: 'b1', col2: 'b2', col3: 'b3', col4: 'b4', col5: 'b5', col6: 'b6', col7: 'b7' },
];
flushGrid(grid);
// The edit should be stopped because the item identity changed
expect(getCellEditor(cell)).to.not.be.ok;
});There was a problem hiding this comment.
Updated it to use __parentRow to be safe. The test from the other issue should cover this already (editor is removed when cell is removed).
|
| }); | ||
|
|
||
| // Focus button mode that is active on MacOS causes issues with Tab key navigation in Firefox when run with Playwright | ||
| (isFirefox && isMac ? it.skip : it)('should navigate through editable cells with Tab', async () => { |
There was a problem hiding this comment.
Had to skip this one on FF / Mac, looks like there is an unrelated issue with the focus button mode in keyboard navigation mixin that can also be observed in some scenarios when testing manually on main.
| }); | ||
| }); | ||
|
|
||
| describe('lazy column rendering', () => { |
There was a problem hiding this comment.
Yeah, I recall facing this same limitation in another issue (maybe also related to grid-pro 🤔). I can't see the issue anymore.
* fix: make conditional editing work with lazy columns * fix tests * handle cell editor navigation when using lazy column rendering * make getEventContext work with lazily rendered cells * stop editing when edited cell is scrolled out of view
|
Hi @sissbruecker and @sissbruecker, when i performed cherry-pick to this commit to 24.10, i have encountered the following issue. Can you take a look and pick it manually? |
* fix: make conditional editing work with lazy columns * fix tests * handle cell editor navigation when using lazy column rendering * make getEventContext work with lazily rendered cells * stop editing when edited cell is scrolled out of view
|
Hi @sissbruecker and @sissbruecker, when i performed cherry-pick to this commit to 24.9, i have encountered the following issue. Can you take a look and pick it manually? |
* fix: make conditional editing work with lazy columns * fix tests * handle cell editor navigation when using lazy column rendering * make getEventContext work with lazily rendered cells * stop editing when edited cell is scrolled out of view Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
….0) (#11411) * fix: make conditional editing work with lazy columns (#11405) * fix: make conditional editing work with lazy columns * fix tests * handle cell editor navigation when using lazy column rendering * make getEventContext work with lazily rendered cells * stop editing when edited cell is scrolled out of view * replace scrollToColumn with individual calls --------- Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
4 participants
Description
The conditional editing logic needs to access the row from a cell element and currently uses
cell.parentElement. When using lazy column rendering, cells are created eagerly when rendering rows but are not added to the row yet, so accessingparentElementdoes not return the row reference.This changes the logic to use the
__parentRowproperty, which is set when creating the cell.Fixes vaadin/flow-components#8976
Type of change