RFR: 8314482: TextFlow: TabStopPolicy [v2]
Kevin Rushforth
kcr at openjdk.org
Tue Jun 17 23:03:37 UTC 2025
On Tue, 17 Jun 2025 16:04:51 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> # Tab Stop Policy
>>
>> Andy Goryachev
>>
>> <andy.goryachev at oracle.com>
>>
>>
>> ## Summary
>>
>> Introduce a `tabStopPolicy` property in the `TextFlow` class which, when set, overrides the existing `tabSize`
>> value and provides consistent way of setting tab stops at the paragraph level, regardless of the individual text
>> segments font [0].
>>
>> 
>>
>>
>> ## Goals
>>
>> The goal of this proposal is to provide a better way for controlling tab stops in the `TextFlow` containing rich text.
>>
>>
>>
>> ## Non-Goals
>>
>> The following are not the goals of this proposal:
>>
>> - support for tab stop types (BAR, or DECIMAL), or attributes like `alignment`
>> - support the `leader` property (symbols to fill the empty space before the tab stop)
>> - support for `firstLineIndent` property
>> - deprecate the `TextFlow::tabsize` property
>>
>>
>>
>> ## Motivation
>>
>> The existing `tabSize` property in the `TextFlow` is inadequate for representing tab stops when the content
>> contains text with different font sizes.
>>
>> In addition to that, a rich text editor might require support for user-customizable tab stops, similar to that provided
>> in RTF or MS Word documents.
>>
>>
>>
>>
>> ## Description
>>
>> ### TextFlow
>>
>>
>> /**
>> * {@code TabAdvancePolicy} determines the tab stop positions within this {@code TextFlow}.
>> * <p>
>> * A non-null {@code TabAdvancePolicy} overrides values set by {@link #setTabSize(int)},
>> * as well as any values set by {@link Text#setTabSize(int)} in individual {@code Text} instances within
>> * this {@code TextFlow}.
>> *
>> * @defaultValue null
>> *
>> * @since 999 TODO
>> */
>> public final ObjectProperty<TabStopPolicy> tabStopPolicyProperty() {
>>
>> public final TabStopPolicy getTabStopPolicy() {
>>
>> public final void setTabStopPolicy(TabStopPolicy policy) {
>>
>> /**
>> * The size of a tab stop in spaces.
>> * Values less than 1 are treated as 1. This value overrides the
>> * {@code tabSize} of contained {@link Text} nodes.
>> * <p>
>> + * Note that this method should not be used to control the tab placement when multiple {@code Text} nodes
>> + * with different fonts are contained within this {@code TextFlow}.
>> + * In this case, the {@link #setTabStopPolicy(TabStopPolicy)} should be used instead.
>> *
>> * @defaultValue 8
>> *...
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 56 commits:
>
> - Merge remote-tracking branch 'origin/master' into 8314482.tab.stops
> - cleanup
> - api
> - Merge remote-tracking branch 'origin/master' into 8314482.tab.stops
> - cleanup
> - Merge remote-tracking branch 'origin/master' into 8314482.tab.stops
> - seems to work
> - wired
> - moved
> - Merge remote-tracking branch 'origin/master' into 8314482.tab.stops
> - ... and 46 more: https://git.openjdk.org/jfx/compare/1ea980ea...7acc55db
Initial API comments.
modules/javafx.graphics/src/main/java/javafx/scene/text/TabStop.java line 35:
> 33: * @since 999 TODO
> 34: */
> 35: public class TabStop {
Should this be a record? If not, it should at least be a final class.
modules/javafx.graphics/src/main/java/javafx/scene/text/TabStop.java line 41:
> 39: * Constructs a new tab stop with the specified position.
> 40: *
> 41: * @param position the position in pixels
Is pixels the right unit here? Should it be points instead? Or should it allow for the specification of units so that you could specify it in "em"s?
@prrace might want to weigh in on this.
modules/javafx.graphics/src/main/java/javafx/scene/text/TabStopPolicy.java line 39:
> 37: * @since 999 TODO
> 38: */
> 39: public class TabStopPolicy {
Is there a reason this class is not final? It probably should be.
modules/javafx.graphics/src/main/java/javafx/scene/text/TabStopPolicy.java line 51:
> 49: /**
> 50: * Specifies the unmodifiable list of tab stops, sorted by position from smallest to largest.
> 51: * The list can be changed using
"changed using" ... what?
More importantly, this isn't an unmodifiable list, right? (it isn't implemented that way, and it would not be very useful if it were) So you can't enforce that it is sorted. Rather this should just be an ordinary observable list, like we have in other classes, and if you need a sorted view of it, then have one internally that keeps it sorted.
modules/javafx.graphics/src/main/java/javafx/scene/text/TabStopPolicy.java line 60:
> 58:
> 59: /**
> 60: * Provides default tab stops (beyond the last tab stop specified by {@code #tabStops()},
Having this be a plural when it is a single value (a double) is awkward. I would name the property `defaultStop` and document that the default is used for all stops beyond the last stop.
modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java line 569:
> 567:
> 568: /**
> 569: * {@code TabAdvancePolicy} determines the tab stop positions within this {@code TextFlow}.
`TabAdvancePolicy` --> `TabStopPolicy`
modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java line 613:
> 611: @Override
> 612: public String getName() {
> 613: return "tabAdvancePolicy";
`"tabStopPolicy"`
-------------
PR Review: https://git.openjdk.org/jfx/pull/1744#pullrequestreview-2937257228
PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2153291538
PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2153291078
PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2153293418
PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2153296860
PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2153303020
PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2153281868
PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2153284183
More information about the openjfx-dev
mailing list