On Thu, 24 Mar 2022 06:14:42 GMT, Robert Lichtenberger <rlich...@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. > > Robert Lichtenberger has updated the pull request incrementally with one > additional commit since the last revision: > > 8251480: TableColumnHeader: calc of cell width must respect row styling > > Old behaviour restored for table rows with custom skins. > Unnecessary call to cell.applyCss() removed. Just commented some minor things. But overall it looks good. I did some manual tests as well and can confirm that this works. :)  modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java line 715: > 713: tableRow = createMeasureRow(tv, tableSkin, null); > 714: } > 715: assert tableRow.getSkin() instanceof SkinBase<?>; Not sure if there is some convention about code asserts, maybe some else can tell. At least I never saw one in a PR here. modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java line 276: > 274: } > 275: > 276: private TableRow<Person> createSmallRow(TableView<Person> tableView) > { Minor: Move this method to the other row method below? modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java line 295: > 293: private TableRow<Person> createCustomRow(TableView<Person> > tableView) { > 294: TableRow<Person> row = new TableRow<>() { > 295: protected javafx.scene.control.Skin<?> createDefaultSkin() { Very minor: `javafx.scene.control.` is not needed ------------- Marked as reviewed by mhanl (Author). PR: https://git.openjdk.java.net/jfx/pull/757
