[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-openjfx-dev
Subject: Re: RFR: 8231644: TreeTableView Regression: Indentation wrong using Label as column content type [v3
From: Marius Hanl <mhanl () openjdk ! java ! net>
Date: 2021-07-27 5:45:50
Message-ID: 24XaRxgrUri2EYi_cdS5y1i3BljInwnBafxkvC7EgLg=.bd3b0dc8-c70b-49b9-a2d5-d1dc9aa3eb9f () github ! com
[Download RAW message or body]
> This PR fixes a long standing issue with the TreeTableView indentation.
>
> ![image](https://user-images.githubusercontent.com/66004280/124681647-473e7380-dec9-11eb-906d-4228fc39cbf9.png)
>
> In short:
> **TreeTableCellSkin** overrides **leftLabelPadding()** to calculate the indentation \
> (the result of this method is added to **x**). While this is fine, this method is \
> not always called (by **LabeledSkinBase#layoutLabelInArea**), e.g. when no text is \
> set. So when a TreeTableCell only sets a graphic (e.g. via **setGraphic()** in \
> **updateItem()**), the indentation will be messed up.
> Fixed this by adding the calculated indentation to **x** before we call \
> **layoutChildren()**.
> We also need/should adjust every other location where `leftLabelPadding()` was \
> used:
> - **computePrefHeight** -> prefWidth() is always called with -1, so nothing got \
> broken by not overriding this, but we should do it of course to be accurate in case \
> we do one day.
> - **computePrefWidth** -> this is needed for auto sizing. I saw that it was \
> slightly off without, so this 100% needed.
> - **computeMinWidth** -> the min width of a cell is not used, so nothing got broken \
> by not overriding this but same as in computePrefHeight(), we should comply with \
> the specs.
> - **layoutChildren** -> I saw a slight off sizing if the indentation is not \
> subtracted to the width.
> As a result of this, all method do effectively the same as they did with an \
> overridden `leftLabelPadding()` (just earlier/later).
> Note: I also added some tests which pass before and pass after.
Marius Hanl has updated the pull request with a new target base due to a merge or a \
rebase. The pull request now contains three commits:
- Merge branch 'master' of https://github.com/openjdk/jfx into 8231644-indentation
Conflicts:
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableCellSkin.java
- calculated indentation in every method now which was previously done via \
leftLabelPadding
- 8231644: fixed wrong indentation for tree cells with graphic only
-------------
Changes: https://git.openjdk.java.net/jfx/pull/568/files
Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=568&range=02
Stats: 211 lines in 2 files changed: 197 ins; 0 del; 14 mod
Patch: https://git.openjdk.java.net/jfx/pull/568.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/568/head:pull/568
PR: https://git.openjdk.java.net/jfx/pull/568
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic