RFR: JDK-8313628: Column drag header, overlay and line are not correctly aligned
Andy Goryachev
angorya at openjdk.org
Thu Aug 3 23:07:38 UTC 2023
On Wed, 2 Aug 2023 16:36:47 GMT, Marius Hanl <mhanl at openjdk.org> wrote:
> When a table has padding or the `layoutChildren` method inside the table skin is overridden (and x/y are modified), the drag drag header, column overlay and column line are not correctly aligned.
>
> The reason is that the positions were calculated incorrectly.
> - **Column overlay and column line**
> Always calculate in the x and y from the table. The x and y variables contain the snapped insets (padding) and possible modifications from subclasses.
> - **Drag header**
> Calculate the drag x offset local bounds from the parent header (which is either the parent column header or the root header)
> Before, the local bounds were calculated from the table, which will wrongly calculate in the padding.
> We do not want to know the local bounds based of the whole table but of our header we are in.
Looks good on macOS with 2 monitors (scales 100% and 200%), and on Windows 11 at different scales, incl. fractional: 100% and 225%
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableViewSkinTest.java line 79:
> 77: @Test
> 78: void testInitialColumnResizeNodePositions() {
> 79: TableView<String> tableView = new TableView<>();
question: would it make sense to test with scene scale(s) other than 100%? just to see if there is no failures with fractional scales?
(the asserts would need to use non-0 tolerance)
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableViewSkinTest.java line 129:
> 127: Toolkit.getToolkit().firePulse();
> 128:
> 129: assertEquals(5, columnResizeLine.getLayoutX());
comparing doubles with 0 tolerance: this is probably ok, since we expect to see whole numbers.
-------------
Marked as reviewed by angorya (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1193#pullrequestreview-1561961550
PR Review Comment: https://git.openjdk.org/jfx/pull/1193#discussion_r1283798454
PR Review Comment: https://git.openjdk.org/jfx/pull/1193#discussion_r1283797210
More information about the openjfx-dev
mailing list