Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wait for editor focusout on Tab to get updated value #7822

Merged
merged 5 commits into from
Sep 24, 2024

Conversation

web-padawan
Copy link
Member

@web-padawan web-padawan commented Sep 19, 2024

Description

Part of #7796

Fixes #7108

Updated the logic on Tab to not call event.preventDefault() and move focus synchronously as this doesn't handle the case when custom editor component is updating its value on blur (this is how e.g. vaadin-date-picker works).

Instead, we now let the browser handle the Tab key press so that the focusout event is caused by that, and wait for it to be fired (unless the event is from vaadin-select-overlay in which case we still handle it manually).

Note, the focus is still moved to the next cell by the logic in _switchEditCell() after the focusout so the keyboard navigation works as expected. I had to update tests from tab() helper to use sendKeys() to verify this fix.

Also added the integration test for custom editor using vaadin-date-picker (could reuse that later to also cover the second part of the issue, which is about value not updated on overlay closing - that would be a separate PR).

UPD: also added the integration test using vaadin-combo-box which was also affected by the issue with Tab.

Type of change

  • Bugfix

@@ -1,3 +1,4 @@
import './not-animated-styles.js';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have "select column" tests in this suite, I though it would be good to disable select animation.

Comment on lines +309 to +312
// Ignore focusout from internal tab event
if (this.__cancelCellSwitch) {
return;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously the flag was only checked on keydown in _switchEditCell method. However, now when we wait for the native focusout it is necessary to also check it here, otherwise the debouncer will stop editing.

@web-padawan web-padawan marked this pull request as ready for review September 19, 2024 12:35
Copy link

sonarcloud bot commented Sep 19, 2024

@vaadin-bot
Copy link
Collaborator

Hi @web-padawan and @web-padawan, when i performed cherry-pick to this commit to 24.4, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 2a08414
error: could not apply 2a08414... fix: wait for editor focusout on Tab to get updated value (#7822)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@oilhands
Copy link

oilhands commented Oct 8, 2024

Is there any chance this will get pulled into Vaadin 23 at some point?

@vaadin-bot
Copy link
Collaborator

Hi @web-padawan and @web-padawan, when i performed cherry-pick to this commit to 23.5, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 2a08414
error: could not apply 2a08414... fix: wait for editor focusout on Tab to get updated value (#7822)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tab key press not committing editing component updates to the row model
5 participants