RFR: 8340852: ScrollPane should not consume navigation keys when it doesn't have direct focus [v2]

[Kevin Rushforth]

On Sat, 5 Oct 2024 22:09:57 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>>> > 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).
>>> 
>>> That will not change, those keys are processed by the focused control. Using Shift + Navigation key to extend selection will keep working as normal and the pane will scroll if necessary, but not because the scroll pane is handling that key (it would have no clue what to do on shift + right) but because the control involved will try to keep the cursor visible.
>> 
>> I said "think of how Shift+Right to extend the selection is implemented in most table views". I never claimed it was handled by the scroll pane or even that it should be. It is typically implemented by allowing an event/action/selector/whatever to bubble up to the enclosing table view to be acted on even if the focus lies on one of its descendants. I was just countering this (new) concept that a control shouldn't act on a keystroke unless it has direct focus. That's just not a generally accepted rule.
>> 
>>> Now, **custom** controls are a different matter. These may have relied on this behavior, and I think that's why this change should be mentioned explicitly in the release notes. They no longer get "scrolling" for free. However, I think even for these controls the default behavior is undesirable. We're again talking about focusable controls, and letting scroll pane have its way will mean those controls can scroll out of view. If it is a single view filling control (like a `*View`) it is likely these keys have been overridden by the custom control to provide better than default behavior.
>> 
>> If I understand you correctly you're once again citing a new rule, namely that PgUp and PgDn shouldn't allow the focused node to scroll out of view.
>> 
>> It is also not a generally accepted rule that if the default behavior isn't correct in all cases it should be removed.
>> 
>> I'm going to tap out of this conversation. I don't agree with these changes but don't think they will inflict much harm. I believe that if there's pushback from the developer community you will not get far using the novel principles you're citing (like the one outlined in the bug title). But perhaps I'm out of step on this and it's time for others to chime in.
>
>> I said "think of how Shift+Right to extend the selection is implemented in most table views". I never claimed it was handled by the scroll pane or even that it should be. It is typically implemented by allowing an event/action/selector/whatever to bubble up to the enclosing table view to be acted on even if the focus lies on one of its descendants. 
> 
> I'm sorry, I misunderstood what you meant here.  This kind of bubbling up is not the problem.  Such events bubble up from the sub-structure of a control (say a table cell) to the table.  The table is still the thing that has focus, which also manages a cursor (the active cell, or the more common cursor in an edit box).  There is no confusion here for users what the focused control is and where input is going, and so it is absolutely fine to let this bubble up to a `Node` that is from a user perspective  "focused" and part of the same control.
> 
> This is not the case with `ScrollPane`. Here it clearly bubbles "out" of the focused area to a different control.  If there was a focused box around the entire control (including the scroll bars, like a TableView, TextArea or ListView have) then it is clear to the user where input is going.
> 
>> I was just countering this (new) concept that a control shouldn't act on a keystroke unless it has direct focus. That's just not a generally accepted rule.
> 
> Perhaps I worded this poorly.  What I always meant to say is a key press should go to the control that has visible focus.  If that control is composed of (to the user irrelevant) other components, then it is fine to let events bubble up within the control and its substructure, as long as the event is not acted upon "outside" the user visible focused area.
> 
> Acting on a key press bubbled up from a focused `Button` to a `ScrollPane` would clearly violate this.  The scroll pane does not have visible focus, the button does.
> 
> Acting on a key press bubbled up from a table cell doesn't violate this; the `TableView` has the focus (and shows this with a visual cue), the cell is just its cursor and is part of the table view's sub-structure.  Here you can see that the `TableView` has the focused border, and so is free to act on key presses that bubble up to that point:
> 
> ![image](https://github.com/user-attachments/assets/fc4b003a-f438-40ca-a511-6fc69f2d56b9)
> 
>> If I understand you correctly you're once again citing a new rule, namely that PgUp and PgDn shouldn't allow the focused node to scroll out of view.
> 
> I'll admit that this just see...

@hjohn I now think this does warrant a CSR, if for no other reason than to capture the outcome of this discussion (once it is finished), and any potentially visible behavioral change that a custom control writer might see. Further, it might be helpful to document this in the `ScrollPane` class docs.

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

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