RFR: 8277000: Tree-/TableRowSkin: replace listener to fixedCellSize by live lookup [v2]

Marius Hanl mhanl at openjdk.org
Tue Dec 10 21:58:43 UTC 2024


On Tue, 10 Dec 2024 21:10:54 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Marius Hanl has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Merge branch 'master' of https://github.com/openjdk/jfx into 8277000-tree-table-row-skin-live-lookup
>>  - Call getFixedCellSize() once
>>  - 8277000: Tree-/TableRowSkin: replace listener to fixedCellSize by live lookup
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java line 421:
> 
>> 419:             } else {
>> 420:                 width = tableCell.prefWidth(height);
>> 421:                 // we only add/remove to the scenegraph if the fixed cell
> 
> are you sure this is correct?
> the tableCell is added in L343 only if `fixedCellSize > 0`
> the removal in L424 misses that logic

Yes, IntelliJ actually gave me the hint.
`isVisible` can only be `false`, when a `fixedCellSize` is set. So the else branch can only ever be executed when `fixedCellSize > 0`

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1645#discussion_r1878932051


More information about the openjfx-dev mailing list