[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Integrated: 8231644: TreeTableView Regression: Indentation wrong using Label as column content type
From:       Marius Hanl <mhanl () openjdk ! java ! net>
Date:       2021-09-29 15:32:32
Message-ID: cNlokzz__5aA5nUuHOoRMkXLV6GInfh7B1_emFD3IWc=.bf3d5948-7cfb-40d5-979a-7d94221adeb1 () github ! com
[Download RAW message or body]

On Wed, 7 Jul 2021 00:25:15 GMT, Marius Hanl <mhanl@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.

This pull request has now been integrated.

Changeset: 30f56069
Author:    Marius Hanl <mhanl@openjdk.org>
Committer: Kevin Rushforth <kcr@openjdk.org>
URL:       https://git.openjdk.java.net/jfx/commit/30f56069dacf32e43e2c98d05ba252ffc4bd9fe9
                
Stats:     210 lines in 2 files changed: 197 ins; 0 del; 13 mod

8231644: TreeTableView Regression: Indentation wrong using Label as column content \
type

Reviewed-by: aghaisas, kcr

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

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


[prev in list] [next in list] [prev in thread] [next in thread]