On Fri, 6 Nov 2020 23:33:08 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

>> As discussed in the JBS 
>> [issue](https://bugs.openjdk.java.net/browse/JDK-8177945), there are some 
>> inconsistencies in the use of `VirtualContainerBase::requestRebuildCells` 
>> from `VirtualContainerBase::updateItemCount()`, which is implemented in the 
>> different skin classes for virtualised controls `TableViewSkinBase`, 
>> `ListViewSkin` or `TreeTableViewSkin`.
>> 
>> The latter already commented out this call (related to JDK-8155798 and 
>> JDK-8147483). 
>> 
>> This PR removes now the calls to `VirtualContainerBase::requestRebuildCells` 
>> from `TableViewSkinBase`  (except for the case `itemCount = 0` based on 
>> JDK-8118897 and JDK-8098235) and `ListViewSkin`.
>> 
>> A test is provided for TableView, that verifies that the `selected` 
>> pseudo-class state remains set for the selected cell while adding more 
>> items. Without this fix, as the cells are rebuilt, the pseudo-class states 
>> are clean and set all over again, thus the flickering.
>> 
>> For ListView, the test rt_35395 (JDK-8091726) is updated, as now there are 
>> way less calls to updateItem.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListViewSkin.java
>  line 359:
> 
>> 357: 
>> 358:         updatePlaceholderRegionVisibility();
>> 359:         if (newCount == oldCount) {
> 
> Does this also need the same `else if (oldCount == 0)` test that you added to 
> `TableViewSkinBase`?

`TableViewSkinBase` has an `itemsChangeListener` that triggers the 
`rowCountListener`, which sets `itemCount = 0` when an item has been replaced. 
`ListViewSkin` uses a different approach via `listViewItemsListener` -> 
`flow::setCellDirty`.

However, the same approach (`itemCount = 0`) is used in both cases when the 
list of items is cleared, so I'll add it here as well.

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

PR: https://git.openjdk.java.net/jfx/pull/348

Reply via email to