On Fri, 20 Mar 2026 15:37:23 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 >> >> >> ### Questions to the Reviewers >> >> - should the informational note [0] be added to the repo, and where? maybe >> under `doc-files/controls/RichTextArea` ? >> >> ### References >> >> [0] [Rich Text Area (Incubator) Data Format Version >> 2](https://github.com/andy-goryachev-oracle/jfx/blob/8356042.ruler/doc-files/controls/RichTextArea/RichTextArea_DataFormat_v2.md) >> >> --------- >> - [x] I confirm that I make this contribu... > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last revision: > > add exports Generally looks good. I tested it using the app and it is working as expected. I left a few comments and questions inline. The next step is to file the CSR. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/RichTextArea.java line 2594: > 2592: > 2593: private final void setDocumentArea(double minX, double minY, double > width, double height) { > 2594: if (documentArea != null) { Typically a setter calls the property to create it if not already created. Is there a reason to instead ignore the set call in that case? modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/RichTextFormatHandler.java line 675: > 673: case '%': > 674: // decoding hex > 675: if(sb == null) { Minor: space after `if`. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/RichTextFormatHandler.java line 677: > 675: if(sb == null) { > 676: sb = new StringBuilder(sz); > 677: if(i > 0) { Ditto modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyleAttribute.java line 113: > 111: * @since 27 > 112: */ > 113: public boolean isCharacterAttribute() { Have you considered a 3-valued enum for attribute type rather than 3 separate boolean attributes, which are mutually exclusive? modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyleAttribute.java line 158: > 156: int h = getClass().hashCode(); > 157: h = 31 * h + name.hashCode(); > 158: h = 31 * h + type.hashCode(); Should you also add the attribute type (isCharacterAttribute(), etc)? It seems more bullet-proof, although in practice, it seems unlikely that there would be two attributes with the same name where one was a character attr and the other was a paragraph attr. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledSegment.java line 54: > 52: public enum Type { > 53: /** Identifies a segment which contains the document properties */ > 54: DOCUMENT_PROPERTIES, This needs `@since 27`. modules/jfx.incubator.richtext/src/test/java/test/jfx/incubator/scene/control/richtext/model/TestRichExport.java line 66: > 64: public void simpleTextModel() { > 65: append("1/n2"); > 66: verify("{}1/n2{!}"); Did you mean to test newlines? If so, then you have a typo: `/n` --> `\n`. If you did mean forward-slash + `n` then ok. modules/jfx.incubator.richtext/src/test/java/test/jfx/incubator/scene/control/richtext/model/TestRichTextFormatHandler.java line 290: > 288: public void save() throws IOException { > 289: RichTextModel m = new RichTextModel(); > 290: m.setDefaultTabStops(155); It would be helpful to also test saving / loading tab stops. ------------- PR Review: https://git.openjdk.org/jfx/pull/1800#pullrequestreview-4109135126 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3082665662 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3082435462 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3082437220 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3082514002 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3082521526 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3082641266 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3082671340 PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3082692000
