On Sat, 1 Apr 2023 10:02:26 GMT, Marius Hanl <mh...@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
