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

Andy Goryachev angorya at openjdk.org
Thu Dec 12 20:51:53 UTC 2024


On Thu, 12 Dec 2024 16:40:27 GMT, Kevin Rushforth <kcr at 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 83 commits:
>> 
>>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>>  - legal
>>  - unicode license
>>  - add handler
>>  - review comments
>>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>>  - clamp
>>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>>  - review comments
>>  - review comments
>>  - ... and 73 more: https://git.openjdk.org/jfx/compare/b76c05b9...e5814b21
>
> modules/jfx.incubator.input/src/main/java/jfx/incubator/scene/control/input/InputMap.java line 363:
> 
>> 361:      */
>> 362:     // TODO this should not affect the skin input map, but perhaps place NULL for each found KeyBinding
>> 363:     public void unbind(FunctionTag tag) {
> 
> This needs a different name than the other "unbind" method since the semantics are quite different. Since this removes all key bindings for a given function tag, I recommend renaming it to `removeKeyBindingsFor`, which goes nicely with `getKeyBindingsFor`.
> 
> And shouldn't there be an `unregisterFunction(FunctionTag)` method that is the inverse of `registerFunction(FunctionTag, Runnable)`? Or is that what `restoreDefaultFunction(FunctionTag)` does already?

you are right - no need for a separate `unregister(FunctionTag)`, the `restoreDefaultFunction(FunctionTag)` does the job.

> modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/RichTextArea.java line 168:
> 
>> 166:      * {@link  InputMap#register(jfx.incubator.scene.control.input.KeyBinding, FunctionHandler)}.
>> 167:      */
>> 168:     public static class Tags {
> 
> Should this be `Tag` instead of `Tags`? Each instance is a single `FunctionTag`.

Isn't it a collection of tags?  But you are right, it looks better in the behavior code.

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

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


More information about the openjfx-dev mailing list