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.

Sign up for GitHub

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

Jump to bottom

Characters typed with the click shortcut key get cleared from input #18048

Closed
Tracked by #4718
tomivirkki opened this issue Nov 16, 2023 · 3 comments
Closed
Tracked by #4718

Characters typed with the click shortcut key get cleared from input #18048

tomivirkki opened this issue Nov 16, 2023 · 3 comments
Assignees
@tltv
Labels
prerelease version for Vaadin 23.4.0 prerelease version for Vaadin 24.3.0 regression Released with Vaadin 23.4.0.alpha1 Released with Vaadin 24.3.0.alpha5

Comments

@tomivirkki
Copy link
Member

tomivirkki commented Nov 16, 2023 •
edited
Loading

Description of the bug

There seems to be some click shortcut related regression.

With the following view:

var button = new Button("Click me");
button.addClickShortcut(Key.KEY_X).setBrowserDefaultAllowed(true);
add(button, new TextField());

(the outcome is the same with NativeButton and Input)

For some reason, every second character following a press of the click shortcut key gets cleared:

Kapture.2023-11-16.at.15.28.00.mp4

Related issue: #10362

Expected behavior

The characters should not get cleared from the input:

Kapture.2023-11-16.at.15.26.49.mp4

Minimal reproducible example

See description above

Versions

  • Vaadin / Flow version: 24.3-SNAPSHOT
@tomivirkki tomivirkki mentioned this issue Nov 16, 2023
Merged
@tltv
Copy link
Member

tltv commented Nov 17, 2023

Using following instead would fix that:

button.addClickShortcut(Key.KEY_X).allowBrowserDefault().setResetFocusOnActiveElement(false);

It calls setResetFocusOnActiveElement(true) by default in ClickNotifier#addClickShortcut. But Flow team has decided to revert this default behavior back to the previous as it may cause side effects for some after all.

@tltv tltv self-assigned this Nov 17, 2023
@tltv tltv moved this to ⚒️ In progress in Vaadin Flow ongoing work (Vaadin 10+) Nov 17, 2023
tltv added a commit that referenced this issue Nov 17, 2023
@tltv
fix: revert addClickShortcut to not reset focus by default
e398183 
Removes a call `setResetFocusOnActiveElement(true)` in `addClickShortcut` method in `ClickNotifier` to revert default behavior changes from #17826.

Fixes: #18048
@github-project-automation github-project-automation bot moved this from ⚒️ In progress to Done in Vaadin Flow ongoing work (Vaadin 10+) Nov 17, 2023
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.3.0.alpha5 and is also targeting the upcoming stable 24.3.0 version.

tltv added a commit that referenced this issue Nov 24, 2023
@tltv
fix: revert addClickShortcut to not reset focus by default
3875ad7 
Removes a call `setResetFocusOnActiveElement(true)` in `addClickShortcut` method in `ClickNotifier` to revert default behavior changes from #17826.

Fixes: #18048
@ZheSun88 ZheSun88 mentioned this issue Nov 28, 2023
Open
12 tasks
mshabarov added a commit that referenced this issue Nov 30, 2023
@tltv @mshabarov
feat: reset focus on active element in ShortcutRegistration (#18126)
09e4737 
* feat: reset focus on active element in ShortcutRegistration

Adds setResetFocusOnActiveElement, resetFocusOnActiveElement and isResetFocusOnActiveElement to ShortcutRegistration class to optionally reset focus on active element (focused element in browser). Resetting means calling blur() and focus() on the active element to ensure that input fields with ValueChangeMode ON_CHANGE will fire value change event before shortcut handler is run.
Updates ClickNotifier#addClickShortcut(Key,KeyModifier...) to reset focus on active element by default for click shortcuts.

Fixes: #5959

* test: fixed tests

* chore: added null checking and text changes

* chore: added window.Vaadin.Flow.resetFocus function

* fix: revert addClickShortcut to not reset focus by default

Removes a call `setResetFocusOnActiveElement(true)` in `addClickShortcut` method in `ClickNotifier` to revert default behavior changes from #17826.

Fixes: #18048

* chore: update Reset focus on active element to work in 23

Adjusted reset focus script snippet from Flow 24 to work with 23.

* chore: formatting

---------

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
@tltv tltv mentioned this issue Dec 19, 2023
Closed
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.4.0.alpha1 and is also targeting the upcoming stable 23.4.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tltv tltv

Labels
prerelease version for Vaadin 23.4.0 prerelease version for Vaadin 24.3.0 regression Released with Vaadin 23.4.0.alpha1 Released with Vaadin 24.3.0.alpha5
Projects
Vaadin Flow ongoing work (Vaadin 10+)
Archived in project
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tomivirkki @tltv @vaadin-bot