Framework: Problem with ShortCuts of same components

[vaadin-bot]

Originally by _Palamarchuk Vladimir_

---

I have a panel with TextField. When user press Enter I have to do something. There are more then one panels in application. I have problem - TextField from first Panel intercept ENTER from all TextFields. See attachments.

---

Imported from https://dev.vaadin.com/ issue #15122

[vaadin-bot]

Originally by _Palamarchuk Vladimir_

---

Attachment added: EnterShortCutTest.zip (13.2 KiB)

[vaadin-bot]

Originally by @denis-anisimov

---

This is common design/implementation issue with shortcuts.
Current implementation uses global shortcuts. So it doesn't matter where shortcut is activated (i.e. shortcut keys are pressed). Shortcut could be activated on any component outside of intended target (regardless of "focus").
So current behavior is expected from the implementation point of view: since shortcut is global then only very first shortcut listener is executed (both listeners works for the same key). And target component is passed in the "target" argument of the listener.
But in that case API is confusing. Global shortcuts should be attached to UI instance only. In that case it has sense.
So current API assumes that shortcuts should be context dependent (works only when it's activated inside the component). But it's not.

I'm going to change the implementation in the following way:

• if shortcut is attached to the UI then it will be global shortcut. So it doesn't matter where the shortcut is activated. Shortcut listener for the UI will be invoked.
• if shortcut is attached to some non UI component then shortcut listener for this component will be invoked only when shortcut is activated inside the component (target is either the component itself or some child inside it).

[vaadin-bot]

Originally by @denis-anisimov

---

https://dev.vaadin.com/review/5517

[vaadin-bot]

Originally by @denis-anisimov

---

Unfortunately the API is confusing and not well designed. But I cannot do here much.
Shortcut listeners with existing API contract is used indirectly in other functionality.
So I cannot restrict listener target to get notifications only when shortcut is activated inside the component as I was going to do it originally.
So API assumes that shortcut listener is GLOBAL listener and it will be invoked each time when shortcut is activated regardless of it's target.

So attached application still will work in an unexpected way: both listeners will be invoked when ENTER is pressed (either on the textfields or outside, it doesn't matter).
ShortcutListener.handleAction() second argument provides a target that should be used to identify end target of shortcut.
I would say that shortcut listener should be attached to the UI , not for text field classes since there is no reason to have several listeners. It should be only one listener and it's meaningful to attach to it to the UI instance.

[vaadin-bot]

Originally by @denis-anisimov

---

So the issue which will be fixed here: both shortcut listeners (and any other attached for the same shortcut) will be notified.
At the moment only first attached listener is notified.

Use target (second arg) in ShortcutListener.handleAction() to identify the target.

[vaadin-bot]

Originally by _fane89_

---

When will the change from denis be pulled ?

[vaadin-bot]

Originally by @Legioth

---

Duplicate of #12705.

Shortcut listeners are intended to be global-ish (actually scoped to the closest UI/Panel/Window). Instead of adding separate listeners for each component, you should just add one listener to the desired scope and then in the event handler check which component was targeted and take appropriate actions based on that.

Another approach might be to use the KeyActions add-on that gives you more control over how keyboard events are handled for a specific component: https://vaadin.com/directory#addon/keyactions

[vaadin-bot]

Originally by @denis-anisimov

---

I disagree.

The reason of this ticket is the same. That's true. But example in this ticket shows that one can different shortcut listeners to DIFFERENT components.
In the referenced example different shortcut listeners are added to THE SAME component.

So current behavior is very confusing: it could be very hard to find other place in application where shortcut listener has been added first.

Shortcuts are designed globally. I'm OK with that (not really but I can understand that design). But API is designed bad. Each shortcut listener should be notified about pressed shortcut. This behavior has no any relation global shortcut essence. It's possible to notify any attached listener for shortcut event with current design of shortcut (where one has to check target inside listener).

Provided patch does that. And it keeps shortcut design as is.

[vaadin-bot]

Originally by @Legioth

---

@denis-anisimov

The reason of this ticket is the same. That's true. But example in this ticket shows that one can different shortcut listeners to DIFFERENT components.
In the referenced example different shortcut listeners are added to THE SAME component.

If you wan't different things to happen for different components when pressing the same shortcut key within the same UI/Panel/Window scope, then you're trying to do something that the shortcut action feature is not designed to support. Notifying multiple listeners without also introducing the highly breaking change of only notifying if the specific component was focused will just make everything even more confusing.

In the long term, we should redesign how keyboard events are handled in the framework. Until we get there, there's no point in adding hacks that makes it more convenient to use existing features in ways which they were never intended to be be used.

[vaadin-bot]

Originally by @denis-anisimov

---

I don't insist on any specific solution.

This ticket is about bad API design for shortcut listeners. That's it.
It should be fixed somehow. API doesn't allow to understand the point: shortcut is not assigned to the component but it's considered as assigned to UI/Panel/Window (the first question : why add shortcut listener method exists in every component ? It should be ONLY in UI/Panel/Widnow). And even worse: only one shortcut listener (for given shortcut) will be invoked.
API has to clarify that somehow.
F.e.: Shortcut could be extracted from ShortcutListener and there should be a method

setListener(Shortcut shortcut, ShortcutListener listener);

and this method should not be in AbstractComponent but in scopes ONLY.

Well, this is just a sketch of one possible solution.
This API issue should be fixed somehow. May be later on. And I'm OK with abandoned patch.
But I'm not OK with closing it.

[vaadin-bot]

Originally by [email protected]_

---

I totally agree with denis' last comment. Assuming that the ShortcutListener is scoped at UI/Panel/Window/Whatever-Level then the addShortcutListener()-API-method must be removed from AbstractComponent and added to the specialized component creating the scope.
The current API suggests that you can add a shortcut listener to any component, but that is wrong! The behavior is a different one as it is effectively adding the listener to the outer scope.
Even the Book Of Vaadin 4.4.4 is unclear about that matter. It states that it is possible to add one listener to many components and then distinguish between them by the event's target. But it does not say that you must use only one listener.
So either "repair" the implementation to reflect the current API or on the other hand change the API to reflect the current behavior. But at the moment there is a discrepancy between API, intention and implementation which is unacceptable.

[vaadin-bot]

Originally by _proaccountapp_

---

Updated prioritization date.

[vaadin-bot]

Originally by @thinwire

---

This is such an established API that we can't really go changing or removing it until the next major version.
Yes, we can all agree that the current API is bad. We know we'll need to address this at some point, but it's not an API that we can change until a new major without violating the promises we've made about API compatibility between releases.

[schweinchendick]

Hello again. The new major release happened to be on February this year. The API-change didn't make it into the release. So the bug still exists. What now?

And to be clear: this is not a bug since 2014; it is over 6 years old:
https://vaadin.com/forum#!/thread/77601

[pinonpierre]

Definitively a bad design.

We have an addShortcutListener on each components.
It's really error prone to make the user believe:

• That the shortcutListener is relative to component only (I can live with it)
• Prevent to have the same shortcut on other components (else it's create sporadic issues)

From my point of view at least one thing should be modified:

It's necessary to listen all listeners added. So in other words, there is no reasons to run only one!

I seen that this defined has a bug and it's should be made in the major release but it's time to fix that after 8 years?