RFR: 8301121: RichTextArea Control (Incubator) [v63]

[Kevin Rushforth]

On Tue, 17 Dec 2024 20:01:14 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> modules/jfx.incubator.input/src/main/java/jfx/incubator/scene/control/input/InputMap.java line 292:
>> 
>>> 290:      */
>>> 291:     public void unregister(KeyBinding k) {
>>> 292:         map.put(k, NULL);
>> 
>> Why does this add a `NULL` rather than remove the mapping? I'm trying to understand the difference between this method and `resetKeyBindings` (maybe this is related to my earlier question above?). One thing to consider is that allowing `NULL` values means that there is no way to tell the difference between a key with a `NULL` value and a key that isn't in the map without calling `map.contains`, which you don't expose.
>
> see my previous comment for the use case.
> 
> this method disables the key binding, regardless of whether it's done programmatically by the application (as in constructor of the custom RTA), or the skin.
> 
> if we are talking about implementation, the NULL basically blocks subsequent lookup in the skin input map.

I see. In that case, unregister is not a good name, since it is _not_ the inverse of register. What it is really doing is registering a no-op binding. In which case... why is it needed at all? The application can just as easily do this:


    register(keyBinding, () -> {});


If there really is a compelling reason to keep this method, then we need to think of a better name. Maybe "disable" or "disableKeyBinding"?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1889167616