On Tue, 27 Jul 2021 05:45:50 GMT, Marius Hanl <mh...@openjdk.org> wrote:

>> 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 and always now of 
>> course).
>> 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

Both the fix and test look good.

-------------

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/568

Reply via email to