RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling

[Marius Hanl]

On Wed, 16 Mar 2022 08:20:59 GMT, Robert Lichtenberger <rlichten at openjdk.org> wrote:

> This fix respects a row factory, if present.
> It will put the cell that is used to measure the column width as child below the row.
> In that way the row's style will be used.

The approach looks good. I left some comments and questions

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java line 650:

> 648:         }
> 649:         Callback<TableView<T>, TableRow<T>> rowFactory = tv.getRowFactory();
> 650:         TableRow<T> tableRow = rowFactory != null ? rowFactory.call(tv) : new TableRow<>();

When there is no row factory, we probably should just return like in line 632-633?

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java line 653:

> 651:         tableSkin.getChildren().add(tableRow);
> 652:         tableRow.applyCss();
> 653:         ((SkinBase<?>) tableRow.getSkin()).getChildren().add(cell);

I don't think this is a safe cast. Instead, you probably should do an `instanceof SkinBase` check before

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java line 667:

> 665:             if ((cell.getText() != null && !cell.getText().isEmpty()) || cell.getGraphic() != null) {
> 666:                 tableRow.applyCss();
> 667:                 cell.applyCss();

Just wondering: Is `cell.applyCss();` still needed here?

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java line 259:

> 257:     /**
> 258:      * @test
> 259:      * @bug 8251480 Row style must affect the required column width

This annotations are not needed, although they do not hurt (just a note)

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

Changes requested by mhanl (Author).

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