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

Marius Hanl mhanl at openjdk.java.net
Thu Jul 8 21:26:53 UTC 2021


On Wed, 7 Jul 2021 23:57:31 GMT, Kevin Rushforth <kcr at 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 basically add the indentation to **x** when we call **layoutChildren()** instead of letting **layoutLabelInArea()** do it sometimes via **leftLabelPadding()**. (which is called directly by **layoutChildren()**)
>> 
>> Note: I also added some tests which pass before and pass after.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableCellSkin.java line 107:
> 
>> 105: 
>> 106:     @Override
>> 107:     protected void layoutChildren(double x, double y, double w, double h) {
> 
> This isn't a review, but just a question and a  comment.
> 
> Are there any other callers of `leftLabelPadding` that could be affected by the removal of the override?
> 
> Btw, you should add `/** {@inheritDoc} */` to the overridden `layoutChildren` method, since it is public API.

good point. I didn't saw any difference but now had a deeper look. Looks like there can be cirumstances, where overriding `layoutChildren ` is not enough.

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

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


More information about the openjfx-dev mailing list