On Wed, 24 Aug 2022 22:03:26 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> The issue is caused by TreeTableRow incorrectly selected when cell selection
>> mode is enabled.
>>
>> Changes:
>> - modified TreeTableRow.updateSelection()
>
> Andy Goryachev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8292353: whitespace
modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlUtils.java
line 157:
> 155: }
> 156: throw new Error("TableRow not found at " + row);
> 157: }
no review, just a couple of comments:
Generally, should prefer api over lookup - we have shims to access
package/protected api.
In this particular case, the lookup is brittle: it assumes that
- either there is a single row configured with the given index
- or that it's the first when iterating over the set
With the first being incorrect (corner case, of course :)
@Test
public void testLookupLastNotEmptyCell() {
stageLoader = new StageLoader(table);
int index = table.getItems().size()- 1;
Set<Node> tableRows = table.lookupAll(".table-row-cell").stream()
.filter(n -> n instanceof TableRow<?> && ((TableRow<?>)
n).getIndex() == index)
.collect(Collectors.toSet());
assertEquals(1, tableRows.size());
}
we fall back to the second assumption, which is implementation dependent (and
unspecified) - it's accidental that we actually do get the row we want.
Having done a bit of digging into VirtualFlowTestUtils (see
[JDK-8293356](https://bugs.openjdk.org/browse/JDK-8293356)) I now think it's
safe enough to use it (even though it's on the oldish side, not using recently
added api, probably needs some polish) - as long as we keep the scenegraph
stable during the test, f.i. by manually adding the table to a stage (either
via StageLoader or showing in our own).
-------------
PR: https://git.openjdk.org/jfx/pull/875
