On Fri, 4 Oct 2024 21:19:55 GMT, Martin Fox <m...@openjdk.org> wrote:
> 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). 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. > 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. It's more than that, most FX controls will not let these keys bubble up at all. - None of the `*View` controls let any of these keys bubble up, because the default scroll behavior is not suitable (they want to scroll by rows, or real pages, and so all those keys are handled directly). - Editable fields will consume all keys (yes, a TextField consumes pgup/pgdown even though it has no use for them). This is probably desirable as it would be very annoying to have a wrong key press scroll your pane. - Other FX controls, like Buttons, or controls that have been made focus traversable (like Labels) do let these keys bubble up, but it is probably very questionable to let scroll pane act on them for the simple reason that the focused control can now be scrolled out of view and become invisible... now we're in a state where the user does not know what is focused, and has no idea what their next key press will do (assuming they have a short memory or left the application like this for a while). So for FX controls that have scrolling behavior baked in, nothing changes. For controls that do not, either they all consume those keys or the scroll behavior you get by default is highly undesirable as it leaves the user guessing what has focus. 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. However, to say no custom controls will be unimpacted by this change is a stretch. There may indeed be custom controls that liked and desired the standard scroll pane behavior. These will need to be modified to capture these keys themselves and provide appropriate scrolling. This is easy to do however. > If nothing else we should make removing PgUp, PgDn, Home, End, and Space a > separate issue. We can do that, if the above hasn't convinced you. We'd be rehashing all these points likely again though: - Default behavior is likely wrong (pane is unaware of what is displayed, and will make odd jumps) - Default behavior can scroll view away from focused control, leaving the user guessing what their key presses will do next once focused control becomes invisible - Default behavior is overridden in all FX controls that offer scrolling - Default behavior is likely overridden in custom controls where scrolling makes sense to get better scroll positioning - You still get the default behavior when displaying something that can't get focus (ie. a map, or an image, or a big piece of text) if of course you allowed the scroll pane to get focus in the first place I also checked some controls outside FX. In ControlsFX at least they seem to rely on internal skins for most of their scrollable controls, and require an `add-opens` to make use of the library with FX. This means they are likely not impacted by this change either. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1582#issuecomment-2395009430
