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

[Lukasz Kostyra]

On Mon, 2 Dec 2024 20:00:13 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Incubating a new feature - rich text control, **RichTextArea**, intended to bridge the functional gap with Swing and its StyledEditorKit/JEditorPane. The main design goal is to provide a control that is complete enough to be useful out-of-the box, as well as open to extension by the application developers.
>> 
>> This is a complex feature with a large API surface that would be nearly impossible to get right the first time, even after an extensive review. We are, therefore, introducing this in an incubating module, **jfx.incubator.richtext**. This will allow us to evolve the API in future releases without the strict compatibility constraints that other JavaFX modules have.
>> 
>> Please check out two manual test applications - one for RichTextArea (**RichTextAreaDemoApp**) and one for the CodeArea (**CodeAreaDemoApp**). Also, a small example provides a standalone rich text editor, see **RichEditorDemoApp**.
>> 
>> Because it's an incubating module, please focus on the public APIs rather than implementation.  There **will be** changes to the implementation once/if the module is promoted to the core by popular demand.  The goal of the incubator is to let the app developers try the new feature out. 
>> 
>> **References**
>> 
>> - Proposal: https://github.com/andy-goryachev-oracle/Test/blob/main/doc/RichTextArea/RichTextArea.md
>> - Discussion points: https://github.com/andy-goryachev-oracle/Test/blob/main/doc/RichTextArea/RichTextAreaDiscussion.md
>> - API specification (javadoc): https://cr.openjdk.org/~angorya/RichTextArea/javadoc
>> - RichTextArea RFE: https://bugs.openjdk.org/browse/JDK-8301121
>> - Behavior doc: https://github.com/andy-goryachev-oracle/jfx/blob/8301121.RichTextArea/doc-files/behavior/RichTextAreaBehavior.md
>> - CSS Reference: https://cr.openjdk.org/~angorya/RichTextArea/javadoc/javafx.graphics/javafx/scene/doc-files/cssref.html
>> - InputMap (v3): https://github.com/andy-goryachev-oracle/Test/blob/main/doc/InputMap/InputMapV3.md
>> - Previous Draft PR: https://github.com/openjdk/jfx/pull/1374
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 72 commits:
> 
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - whitespace
>  - since 24
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - hide skin input map
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - whitespace
>  - ... and 62 more: https://git.openjdk.org/jfx/compare/67eed6d8...4b27b891

Got a few comments/questions for you from my section of RTA (jfx.incubator.scene.control.richtext and jfx.incubator.scene.control.richtext.skin), most are minor.

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/RichTextArea.java line 392:

> 390:      * This is a {@link StyleableProperty} with the name {@code -fx-caret-blink-period}.
> 391:      * <p>
> 392:      * alternative:<br>

I see an `alternative:` here. I suspect it's a good time to pick one version for the docs =) (either is fine)

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/RichTextArea.java line 1193:

> 1191:      * @see RichTextArea.Tags#CUT
> 1192:      */
> 1193:     public void cut() {

We have `copy(DataFormat)` and `paste(DataFormat)`. Would it make sense to also have `cut(DataFormat)`?

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/SelectionModel.java line 74:

> 72:      * @param pos the new caret position, must be non-null
> 73:      */
> 74:     public void extendSelection(StyledTextModel model, TextPos pos);

Kind of picky-thinking of nomenclature here (but do correct me if I'm wrong) - "extend" strictly means to make something larger, while this assumes you can also make the selection smaller.

How does `modifySelection` or `adjustSelection` sound like instead?

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/SingleSelectionModel.java line 88:

> 86:             a = pos;
> 87:         } else {
> 88:             if(pos.compareTo(sel.getMin()) < 0) {

Minor: styling mistake - `s/if(/if (` here and L91

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

PR Review: https://git.openjdk.org/jfx/pull/1524#pullrequestreview-2472964042
PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1867884183
PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1867901791
PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1867710940
PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1867676038