On Sun, 6 Sep 2026 19:36:30 GMT, Marius Hanl <[email protected]> wrote:
>> Michael Strauß has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review comments
>
> modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line
> 1992:
>
>> 1990: double alt = -1;
>> 1991:
>> 1992: if (availableHeight != -1 && child.isResizable() &&
>> child.getContentBias() == Orientation.VERTICAL) { // width depends on height
>
> Minor, preexisting: Inline comments always are bit weird, especially here
> where it is very much on the right. Maybe just move the comment `// width
> depends on height` above the method?
>
> (There is another one below as well)
I've removed those comments, they were low-quality in the first place.
> modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line
> 2860:
>
>> 2858: Insets margin, boolean fillWidth,
>> boolean fillHeight,
>> 2859: HPos halignment, VPos valignment,
>> boolean isSnapToPixel) {
>> 2860: //
>
> Very minor: This line can be removed, empty comment line
I've moved the comments closer to where they are actually relevant instead.
> modules/javafx.graphics/src/test/java/test/javafx/scene/layout/RegionTest.java
> line 1569:
>
>> 1567:
>> 1568: assertEquals(10, RegionShim.computeChildMaxAreaWidth(
>> 1569: pane, child, -1, Insets.EMPTY, 50, true));
>
> Minor: The assertion will look better when
> `RegionShim.computeChildMaxAreaWidth` is assigned to a variable before, maybe
> `var width = ...`
Yes in theory, but this assertion style is used approximately five million
times in this test class, so I'm leaving it at that...
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2278#discussion_r3945389329
PR Review Comment: https://git.openjdk.org/jfx/pull/2278#discussion_r3945388815
PR Review Comment: https://git.openjdk.org/jfx/pull/2278#discussion_r3945388062
