On Mon, 28 Oct 2024 16:21:25 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> Andy Goryachev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> review comments > > modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 10458: > >> 10456: case RIGHT -> TraversalDirectionInternal.RIGHT; >> 10457: case UP -> TraversalDirectionInternal.UP; >> 10458: default -> null; > > Do we indent labels inside a switch? I find it more readable since `switch` > opens a new context. This is also what I remember seeing in OpenJFX. Don't > know if it's enforced. I don't indent case labels inside of switch. I don't think we enforce one way or the other. And you can't ask to make it more compact and then complain that it's more compact /jk > modules/javafx.graphics/src/main/java/javafx/scene/TraversalDirection.java > line 40: > >> 38: /** Indicates a focus change to the next focusable Node, possibly >> traversing into the children of the current parent. */ >> 39: NEXT, >> 40: /** Indicates a focus change to the previous focusable node. */ > > `NEXT` talks about traversing children, does `PREVIOUS` need to mention it > too? Could it traverse out of the children of the current parent, or is it > not symmetric? I think the whole "children" thing can be dropped, since there is no NEXT_IN_LINE in the public API anymore. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819380921 PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819374705
