On Thu, 21 Nov 2024 19:24:44 GMT, Kevin Rushforth <k...@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 66 commits: >> >> - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea >> - hide skin input map >> - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea >> - whitespace >> - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea >> - save as >> - removed function handler >> - removed add handler last >> - use focus traversal api >> - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea >> - ... and 56 more: https://git.openjdk.org/jfx/compare/3a8a5598...e45be7b7 > > modules/jfx.incubator.input/src/main/java/jfx/incubator/scene/control/input/KeyBinding.java > line 36: > >> 34: >> 35: /** >> 36: * Key binding provides a way to map key event to a hash table key for >> easy matching. > > What, exactly, is the purpose of this class as public API? The above makes it > sound like an implementation detail. > > Also, what does it provide that KeyMapping doesn't? `KeyMapping` is a totally different beast designed for a different purpose. `KeyBinding` is more like `KeyCombination`, except we cannot use KeyCombination because: - it does not capture KEY_PRESSED/KEY_TYPED/KEY_RELEASED property - it has a sole KeyCombination(String) constructor which requires parsing - it's got a narrow and weird API, unusable for our purpose Also, this hints at the fact that the documentation needs to be improved, thanks. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1853027690