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

Fri Sep 6 22:11:17 UTC 2024

On Thu, 5 Sep 2024 22:25:19 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 incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision:
> 
>  - fixes and review comments
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - improved vertical scrolling
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - cleanup
>  - navigation
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - whitespace
>  - update + review comments
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - ... and 2 more: https://git.openjdk.org/jfx/compare/5e2143b1...bc1615c0

Tha change in StyledTextModel from userEditable --> writable looks good to me. I left a few related javadoc comments and one question.

I wrote a very simple test, something you might see in a "HelloRichTextArea", and it seems quite easy to get started. It did prompt a couple quick suggestions:

`RichTextArea`:

* Do you think it is worth adding a convenience method `appendText(String)` that calls `appendText(String, StyleAttributeMap.EMPTY)`?

`StyleAttributeMap`:

* `fromInlineStyle(String style)`

This method seems redundant:  `fromInlineStyle(style)` and `fromStyles(style)` are equivalent. Do we need both?

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

> 1028:      * sequences result in a new paragraph being added.
> 1029:      * <p>
> 1030:      * This method is no-op if either the control or the model is not editable.  It is up to the model

This method doesn't depend on the editable state of the control at all, and will now throw and exception if the module is null or not writable.

This applies to the other programmatic editing methods, too: `appendText(StyledInput)`, `insertText()`, `replaceText()`, `applyStyle()`, `setStyle()`, and `clear()`.

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

> 1074:      * When selection exists, deletes selected text.  Otherwise, deletes the character preceding the caret,
> 1075:      * possibly breaking up the grapheme clusters.
> 1076:      * This method does nothing if either control or the model is not editable, or the caret position is {@code null}.

Suggestion for here and similar methods:

"does nothing if the control is not editable or the model is not writable..."

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

> 1086:     /**
> 1087:      * Clears the document, creating an undo entry.
> 1088:      * This method does nothing if either control or the model is not editable.

Remove this sentence and add the needed `@throws`

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

> 1141:      * removing the current selection.
> 1142:      * <p>
> 1143:      * This method does nothing if either control or the model is not editable, or the caret position is {@code null}.

As implemented, this method will do the copy, but not the associated delete. Is this intentional? If so, the spec should be changed to clarify that (else the spec is correct and the impl is wrong).

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

> 2094:      * might be wider than one specified.
> 2095:      * <p>
> 2096:      * This method does nothing if either control or the model is not editable.

Remove this sentence.

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

PR Review: https://git.openjdk.org/jfx/pull/1524#pullrequestreview-2287196466
PR Comment: https://git.openjdk.org/jfx/pull/1524#issuecomment-2334872667
PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1747749446
PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1747747945
PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1747754883
PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1747757825
PR Review Comment: https://git.openjdk.org/jfx/pull/1524#discussion_r1747752042