On Mon, 2 Dec 2024 20:00:13 GMT, Andy Goryachev <ango...@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
