On Mon, 24 Jun 2024 23:29:07 GMT, Andy Goryachev <[email protected]> wrote:
>> Jose Pereda has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add tests for all four sides > > modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java > line 947: > >> 945: case BOTTOM: >> 946: // Consider vertical scroll events (dy > dx) >> from mouse wheel and trackpad, >> 947: // and horizontal scroll events from a trackpad >> (dx > dy) > > 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? (sorry I missed to reply) I believe the comment already states what are we taking into account, but this is a more detailed explanation: - a mouse wheel only has vertical scroll events (`dx == 0`, so `abs(dy) > abs(dx)`), then we just take `dy` to produce the horizontal scroll offset. - a trackpad has both scroll events, and we take whichever is bigger. Typically the user will do a horizontal scrolling if tabs are horizontally laid out, and therefore abs(dy) <<< abs(dx), so we take `dx` to produce the horizontal scroll offset, ignoring the smaller `dy` (that wouldn't have any effect in any case). I'm happy to reword the comment if needed. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1486#discussion_r1689397250
