On Wed, 6 Nov 2024 11:37:07 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> Andy Goryachev has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 52 commits: >> >> - readme >> - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea >> - review comments >> - input map >> - validate model >> - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea >> - javadoc >> - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea >> - review comments >> - measurement node >> - ... and 42 more: https://git.openjdk.org/jfx/compare/bd4bc057...a51ae151 > > modules/jfx.incubator.input/src/main/java/jfx/incubator/scene/control/input/KeyBinding.java > line 97: > >> 95: >> 96: /** >> 97: * Utility method creates a KeyBinding corresponding to a key press. > > Minor: typo / rewording. Similar comment applies to other helper methods of > this class. > `Utility method creates` -> `Utility method that creates` OR `Utility > method to create` re-phrased in all related methods, thanks! > modules/jfx.incubator.input/src/main/java/jfx/incubator/scene/control/input/KeyBinding.java > line 257: > >> 255: */ >> 256: public boolean isCommand() { >> 257: return modifiers.contains(KCondition.COMMAND); > > Could safe guard with `if (PlatformUtil.isMac())` check to `return false` if > not a mac platform I wonder if these is* are needed at all... ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1833340978 PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1833342063
