RFR: JDK-8305248: TableView not rendered correctly after column is made visible if fixed cell size is set [v2]

[Jose Pereda]

On Sat, 1 Apr 2023 10:02:26 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> The determined `prefWidth` of a `TableCell` could be `0.0` when a `fixedCellSize` is set.
>> This happened because the `TableCell` may not have a skin since it was never added to the scene graph yet.
>> 
>> The fix is to make sure we get the `prefWidth` after the `TableCell` was added to the scene graph.
>> That is also the reason why the problem only happened the first time and never again after (skin is then already created).
>
> Marius Hanl has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - JDK-8305248: Added the tests also for TreeTableRow
>  - JDK-8305248: Improve comments

Looks good, I have just two minor comments.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java line 356:

> 354:                     getChildren().add(tableCell);
> 355:                 }
> 356:                 // Note: prefWidth() has to be called only after the tableCell is added to the tableRow, if it wasn't

Minor: there is no explicit rule about limiting line comments to 80 characters, as far as I know, but it might be good to apply it to your comments (also to the test ones)

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java line 260:

> 258:      */
> 259:     @Test
> 260:     public void testMakeInvisibleColumnVisible() {

These two tests also pass with/without your patch, but I guess we want to have them to prevent any future issue?

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

PR Review: https://git.openjdk.org/jfx/pull/1077#pullrequestreview-1368489198
PR Review Comment: https://git.openjdk.org/jfx/pull/1077#discussion_r1155611209
PR Review Comment: https://git.openjdk.org/jfx/pull/1077#discussion_r1155621366