8334874: Horizontal scroll events from touch pads should scroll the TabPane tabs

[jperedadnr]

This PR considers the horizontal scroll events that can be generated on a trackpad, on an horizontally sided TabPane control (top or bottom), adding to the existing vertical scroll events from mouse wheel and trackpad, to scroll its tabs.

Therefore, scrolling a TabPane will behave in the same way that ScrollBar does regarding those same events and control orientation (vertical/horizontal).

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8334874: Horizontal scroll events from touch pads should scroll the TabPane tabs (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1486/head:pull/1486
$ git checkout pull/1486

Update a local copy of the PR:
$ git checkout pull/1486
$ git pull https://git.openjdk.org/jfx.git pull/1486/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1486

View PR using the GUI difftool:
$ git pr show -t 1486

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1486.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jpereda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[mlbridge]

Webrevs

• 02: Full - Incremental (2c4914ca)
• 01: Full - Incremental (2046b04e)
• 00: Full (9e6b89c6)

[andy-goryachev-oracle]

Considering that this is an improvement and a change in behavior, I would like to suggest a couple of changes:

1. For Side.LEFT/RIGHT, reverse the direction. What we have now is inconsistent with the movement of e.g. ListView (can be seen if using track pad with the list view and TabPane in the Monkey Tester):

2. Since now the TabPane movement correctly reflects the orientation of track pad gesture, we need to reverse the movement when TabPane effective nodeOrientation is RTL.

3. We don't have the behavior documented currently (some initial docs can be found in /doc-files/behavior). Maybe it's a good opportunity at least start a file for TabPane?

And, a question - does this change require a CSR, since we are changing the behavior?

[kevinrushforth]

And, a question - does this change require a CSR, since we are changing the behavior?

Good question. Thinking out loud, if what this PR does is to change the existing behavior of top/bottom TabPane scrolling (e.g., horizontal scrollbar) to align with what ScrollPane already does, then... maybe not? (a CSR wouldn't be a bad idea, though)

I'd be happy to hear others thoughts on this.

It does need two reviewers.

/reviewers 2

[openjdk]

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[kevinrushforth]

1. For Side.LEFT/RIGHT, reverse the direction. What we have now is inconsistent with the movement of e.g. ListView (can be seen if using track pad with the list view and TabPane in the Monkey Tester):

I don't recommend doing that as part of this PR. That would be a noticeable behavior change, and warrants more discussion.

2. Since now the TabPane movement correctly reflects the orientation of track pad gesture, we need to reverse the movement when TabPane effective nodeOrientation is RTL.

Do ScrollPane or ListView work this way already? If they do, then it seems reasonable to do the same here. Otherwise, I'm not sure we should do this for TabPane in isolation.

[andy-goryachev-oracle]

I don't recommend doing that as part of this PR. That would be a noticeable behavior change, and warrants more discussion

we are changing the behavior already by changing the direction of the gesture, so I thought we should be able to tackle this issue within the same PR.

The other point is the discussion - these comments are already on the mailing list, what alternative discussion is required?

And what are the acceptance criteria for that discussion?

[andy-goryachev-oracle]

Do ScrollPane or ListView work this way already?

Good point - both have the same issue (I am sure other controls as well, TableView, others did not check). Definitely deserves a separate ticket, possibly even an umbrella task (RTL navigation).

[andy-goryachev-oracle]

Maybe we should explain why - to preserve the existing behavior with the mouse (scrolling up/down)?

The trackpad events can have both dx and dy of various magnitude, both non-zero, so the logic seems to me a bit contrived - like filtering some events, although at the end it still feels ok.

What do you think?