On Thu, 8 Jun 2023 00:01:02 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> This PR does two small improvements to `VirtualFlow`.
>> Until now, the `VirtualFlow` sometimes called the `computeHeight` or
>> `computeWidth` methods, although a fixed cell size is set and we therefore
>> don't need to call those method (and never should do, as we may don't get
>> the expected result and mix computed sizes with the fixed cell size).
>>
>> Added tests that fail before and pass now. They check that the
>> `computeHeight` or `computeWidth` (non vertical flow) are never called when
>> a fixed cell size is set.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
> line 1954:
>
>> 1952: * Gets the breadth of a specific cell
>> 1953: */
>> 1954: double getCellBreadth(T cell) {
>
> is there a possibility of introducing a runtime errors?
No, this is just cosmetic and more correct as every cell inside the VirtualFlow
is bound to the type parameter T.
> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java
> line 1579:
>
>> 1577: @Test
>> 1578: public void
>> testComputeWidthShouldNotBeCalledWhenFixedCellSizeIsSet() {
>> 1579: int cellSize = 24;
>
> magic number. setting cell size to 44 fails the test. cellSize=4 is ok.
not per se a magic number in this context, as the VirtualFlow is resized to
300.
So 13 cells fit inside it with this fixed cell size. Making it bigger will of
course fail the test below.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1150#discussion_r1222531553
PR Review Comment: https://git.openjdk.org/jfx/pull/1150#discussion_r1222530483
