On Mon, 14 Sep 2026 14:46:46 GMT, Andy Goryachev <[email protected]> wrote:
>> Updated `KeyCodeCombination.getDisplayText()` to return "NumPad *" text for
>> all numpad keys:
>>
>>
>> Arguments.of("NumPad 0", KeyCode.NUMPAD0),
>> Arguments.of("NumPad 1", KeyCode.NUMPAD1),
>> Arguments.of("NumPad 2", KeyCode.NUMPAD2),
>> Arguments.of("NumPad 3", KeyCode.NUMPAD3),
>> Arguments.of("NumPad 4", KeyCode.NUMPAD4),
>> Arguments.of("NumPad 5", KeyCode.NUMPAD5),
>> Arguments.of("NumPad 6", KeyCode.NUMPAD6),
>> Arguments.of("NumPad 7", KeyCode.NUMPAD7),
>> Arguments.of("NumPad 8", KeyCode.NUMPAD8),
>> Arguments.of("NumPad 9", KeyCode.NUMPAD9),
>> Arguments.of("NumPad *", KeyCode.MULTIPLY),
>> Arguments.of("NumPad +", KeyCode.ADD),
>> Arguments.of("NumPad -", KeyCode.SUBTRACT),
>> Arguments.of("NumPad .", KeyCode.DECIMAL),
>> Arguments.of("NumPad /", KeyCode.DIVIDE)
>>
>>
>> Added test for numpad and also modified the test case where we have
>> platform-specific differences (Backspace, Delete, ...)
>>
>> NOTE: noticed the auto-generated text shows weird names - "Back Space"
>> instead of "Backspace". We might want to double check and fix these as well.
>>
>> some names are weird, perhaps these should also be fixed:
>>
>> KeyCode.BACK_SPACE: Back Space
>> KeyCode.QUOTEDBL: Quotedbl
>> KeyCode.EJECT_TOGGLE: Eject Toggle
>> KeyCode.KP_DOWN: Kp Down
>> KeyCode.KP_LEFT: Kp Left
>> KeyCode.KP_RIGHT: Kp Right
>> KeyCode.KP_UP: Kp Up
>>
>> Also, there is difference in naming certain keys between macOS keyboards and
>> the rest of the world:
>>
>> esc - Esc
>> backspace == delete
>> return - Enter
>> caps lock - Caps Lock
>> shift - Shift
>>
>> The use of symbols for macOS is questionable in my opinion, maybe the
>> keyboard have changed since then:
>>
>> KeyCode.BACK_SPACE: ⌫
>> KeyCode.DELETE: ⌦
>> KeyCode.ESCAPE: ⎋
>>
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Andy Goryachev has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains three additional
> commits since the last revision:
>
> - Merge branch 'master' into 8389582.numpad
> - pgup pgdn esc backspace
> - numpad
I'm fine with the code as-is but did include a few suggestions inline.
modules/javafx.graphics/src/main/java/javafx/scene/input/KeyCodeCombination.java
line 222:
> 220:
> 221: // Returns a suitable string representation of the key code or null
> 222: private static String getSingleChar(KeyCode code) {
It's odd to have a routine named `getSingleChar` that doesn't return single
chars. But I appreciate some odd bits here and there in the code.
modules/javafx.graphics/src/main/java/javafx/scene/input/KeyCodeCombination.java
line 296:
> 294: case PAGE_DOWN: return "PgDn";
> 295: case PAGE_UP: return "PgUp";
> 296: }
This is a matter of style but if it was up to me I would use "Del" instead of
"Delete". "Del" is a common abbreviation and shorter is better for the shortcut
text.
-------------
Marked as reviewed by mfox (Committer).
PR Review: https://git.openjdk.org/jfx/pull/2257#pullrequestreview-5228487068
PR Review Comment: https://git.openjdk.org/jfx/pull/2257#discussion_r4030926220
PR Review Comment: https://git.openjdk.org/jfx/pull/2257#discussion_r4030902963
