On Sat, 11 Oct 2025 12:22:53 GMT, Kevin Rushforth <[email protected]> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java 
>> line 1767:
>> 
>>> 1765:     /**
>>> 1766:      * Calling {@code refresh()} forces the TableView control to 
>>> repopulate the
>>> 1767:      * cells necessary to populate the visual bounds of the control.
>> 
>> the word 'repopulate' is used twice (here and elsewhere)
>> 
>> I would suggest to rephrase these comments to indicate what happens exactly, 
>> possibly borrowing text from the `VirtualFlow`:
>> 
>> - recreate: a layout pass should be done, and that the cell factory has 
>> changed. All cells in the viewport are recreated with the new cell factory.
>> - rebuild: a layout pass should be done, and cell contents have changed. All 
>> cells are removed and then added properly in the viewport
>
> At a minimum, replace the first occurrence of "repopulate" with "rebuild".
> 
> 
>      * Calling {@code refresh()} forces the TableView control to rebuild the
>      * cells necessary to populate the visual bounds of the control.
> 
> 
> I wouldn't over-specify this by saying what `VirtualFlow` will do, but if you 
> want to add a sentence saying that this will request a layout that would be 
> fine:
> 
> 
>      * Calling {@code refresh()} forces the TableView control to rebuild the
>      * cells necessary to populate the visual bounds of the control.
>      * This will request a layout of the TableView cells.

Changed to rebuild. I did not add the request layout line, in case we may want 
to change this later. Since as @hjohn and @johanvos mentioned, it is rather 
weird right now.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1830#discussion_r2426194114

Reply via email to