On Thu, 8 Feb 2024 19:20:32 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> Martin Fox 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 seven additional 
>> commits since the last revision:
>> 
>>  - Consistent terminology and more details in comments.
>>  - Merge remote-tracking branch 'upstream/master' into keypadcombo
>>  - Added SEPARATOR to list of keypad keys
>>  - CharacterCombinations now work on the numeric keypad
>>  - Fixed Monocle
>>  - Merge remote-tracking branch 'upstream/master' into keypadcombo
>>  - Added hint to getKeyCodeForChar to enable numeric keypad
>
> modules/javafx.graphics/src/main/java/com/sun/glass/events/KeyEvent.java line 
> 255:
> 
>> 253:      * character with respect to the currently active keyboard layout or
>> 254:      * VK_UNDEFINED if the character isn't present in the current 
>> layout.
>> 255:      * The hint is the KeyCode of the key the system is attempting to 
>> match.
> 
> is this clear enough or should we say that it's KeyCode.getCode()?
> 
> (also Application:746)

I updated the comment so the wording better matches the existing text. The new 
comment also adds a reminder that the hint could be VK_UNDEFINED.

> modules/javafx.graphics/src/main/java/com/sun/glass/events/KeyEvent.java line 
> 257:
> 
>> 255:      * The hint is the KeyCode of the key the system is attempting to 
>> match.
>> 256:      * It can be used to optimize the search or to distinguish between 
>> the
>> 257:      * main keyboard and the numeric keypad.
> 
> should we mention that it is supposed to be `KeyEvent.VK_UNDEFINED` when not 
> available?

I updated the comment here as well.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1289#discussion_r1484541213
PR Review Comment: https://git.openjdk.org/jfx/pull/1289#discussion_r1484541702

Reply via email to