On Thu, 1 May 2025 18:47:18 GMT, Andy Goryachev <[email protected]> wrote:
> ### Summary > > This PR adds support for controlling tab stops: the `TAB_STOPS` paragraph > attribute and the `defaultTabStops` property in the `RichTextModel`. > > While adding the paragraph attribute is a trivial process, adding a > model-wide property requires adding of a mechanism to support document > properties (something that was originally omitted in the first incubating > release). Using the document properties, we can now persist not only the > default tab stops, but also provide the version umber for the storage format > provided by the `RichTextFormatHandler`, which will enable support the format > evolution in the future [0]. > > To showcase the feature, the `RichEditorDemoApp` gains a visual ruler and > additional dialogs and menus. > > <img width="822" height="381" alt="Screenshot 2026-03-04 at 14 00 30" > src="https://github.com/user-attachments/assets/32251846-f11a-4e87-b74a-44c21d629550"; > /> > > > ### Paragraph Attribute > > Paragraph-specific tab stops are enabled by: > > - `StyleAttributeMap.TAB_STOPS` constant > - `StyleAttributeMap.Builder.setTabStops(double ... positions)` > > ### Default Tab Stops > > After the last paragraph tab stop, or when no paragraph tab stops is set, the > document provides a way to set default tab stops via the model's > `defaultTabStops` property: > > These changes support the new property and other document properties: > > - document-wide properties support in the `StyledTextModel` base class > - `defaultTabStops` property in the `RichTextModel` and > `RichTextFormatHandler` > - document properties `VERSION` and `DEFAULT_TAB_STOPS` > - `StyledSegment`: `ofDocumentProperties()` factory, `getDocumentProperties()` > > ### Other Improvements > > A number of other improvements were made along with the tab stop related > changes: > > - `character()`, `paragraph()`, and `document()` factory methods in the > `StyleAttribute` class > - `isCharacterAttribute()`, `isParagraphAttribute()`, and > `isDocumentAttribute()` methods in the `StyleAttribute` class > - `RichTextArea`: `documentArea` read-only property > > ### Dependencies > > https://bugs.openjdk.org/browse/JDK-8373936 > > ### Questions to the Reviewers > > - should the informational note [0] be added to the repo, and where? > `doc-files/notes/27`? or maybe under `doc-files/info/RichTextArea` ? > > ### References > > [0] [Rich Text Area (Incubator) Data Format Version > 2](https://github.com/andy-goryachev-oracle/Test/blob/8356042.ruler/doc/RichTextArea/RichTextArea_DataFormat_v2.md) Looking at the API, my biggest question is: why is there no `tabStopPolicy` property in the RichTextArea control, which would then be forwarded on to the model? This seems the more natural place to me, unless I'm missing something. I left a few other brief comments on the API inline. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/RichTextModel.java line 77: > 75: * next tab stop computed at regular intervals relative to the > document content leading edge. > 76: * <p> > 77: * A value of equal or less than 0 disables the default interval. And what do tabs do in this case? Fall back to tab stops using characters and render spaces to get to the next stop (like Text and TextFlow)? Render nothing? Render a single space? modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyleAttribute.java line 37: > 35: * @since 24 > 36: */ > 37: public abstract class StyleAttribute<T> { Is there is a compelling reason to make this class abstract? It seem better to keep this as a final concrete class. Subclasses aren't needed (they are overkill) for what you are doing; you can do the same thing with constructor parameters and final fields. If there is some good reason to use subclasses that I'm missing, then I recommend making it a sealed class with named, non-public concrete classes. It isn't meant to be subclassed by applications. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyleAttributeMap.java line 363: > 361: * @return the paragraph alignment attribute value > 362: */ > 363: public TabStop[] getTabStops() { `@since 27` And would it be better to return a `List`? modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledSegment.java line 125: > 123: * @return the properties or null > 124: */ > 125: public Map<String,String> getDocumentProperties() { return null; } `@since 27` ------------- PR Review: https://git.openjdk.org/jfx/pull/1800#pullrequestreview-3938029150 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r2925871964 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r2925935444 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r2925999179 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r2926013447
