On Fri, 4 Oct 2024 11:57:56 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> This change modifies `ScrollPaneBehavior` to only consume keys that are 
>> targetted at it.  As `KeyEvent`s are in almost all cases only intended for 
>> the targetted node (as visually that's where the user expects the keyboard 
>> input to go, as per normal UI rules) consuming key events that bubble up is 
>> simply incorrect.  When the `ScrollPane` is focused directly (it has the 
>> focused border) then it is fine for it to respond to all kinds of keys.
>> 
>> In FX controls normally there is no need to check if a `Control` is focused 
>> (although they probably should **all** do this) as they do not have children 
>> that could have received the Key Events involved, and Key Events are always 
>> sent to the focused Node.  When `ScrollPane` was developed this was not 
>> taken into account, leading to it consuming keys not intended for it.
>> 
>> This fixes several unexpected problems for custom control builders.  A 
>> custom control normally benefits from standard navigation out of the box 
>> (TAB/shift+TAB) and directional keys.  However, this breaks down as soon as 
>> this custom control is positioned within a `ScrollPane`, which is very 
>> surprising behavior and not at all expected.  This makes it harder than 
>> needed for custom control developers to get the standard navigation for the 
>> directional keys, as they would have to specifically capture those keys 
>> before they reach the `ScrollPane` and trigger the correct navigation action 
>> themselves (for which as of this writing there is no public API).
>> 
>> The same goes for all the other keys captured by `ScrollPane` when it does 
>> not have focus, although not as critical as the UP/DOWN/LEFT/RIGHT keys.
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Deduplicate lambda

Again, it’s really, really rare for a ScrollPane to have focus. Making these 
bindings contingent on focus is the same as removing them altogether.

These are behavioral changes and developers will expect a rationale for each 
change. The removal of the arrow key bindings is an easy sell; the bindings 
interfere with traversal. And as we've discovered most toolkits don't implement 
these keys by default.

Two rationales have been presented for removing PgUp, PgDn, Home, and End. One 
is that it’s inappropriate for a node to process a key event when it doesn’t 
have direct focus. This is not a generally accepted rule in any UI toolkit I’ve 
ever worked with (just think of how Shift+Right to extend the selection is 
implemented in most table views). The other is that since the default behavior 
isn’t always correct we should remove it. But that’s true for just about any 
default behavior we provide. These are not strong arguments for removing 
existing behaviors.

If nothing else we should make removing PgUp, PgDn, Home, End, and Space a 
separate issue.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1582#issuecomment-2394686410

Reply via email to