RFR: 8301121: RichTextArea Control (Incubator) [v6]
Andy Goryachev
angorya at openjdk.org
Tue Sep 3 15:59:31 UTC 2024
On Fri, 30 Aug 2024 20:20:22 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>> 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 10 additional commits since the last revision:
>>
>> - 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
>> - whitespace
>> - 8301121: RichTextArea Control (Incubator)
>
> I was looking into what "editable" does in `RichTextArea`, and I think we need a new name for one of the methods and might also need a second convenience method. There are at least three possible states that we need to consider:
>
> A. The model supports editing; the control's editable property is true
>
> This means that the document can be modified via the UI; it can be modified by calling control methods from the app (or by calling the editing methods on StyledTextModel, but apps don't typically do that).
>
> B. The model supports editing, the control's editable property is false
>
> This means that the document cannot be modified via the UI; it _can_ be modified by calling control methods from the app (or by calling the editing methods on StyledTextModel, but apps don't typically do that).
>
> C. The model does not support editing; it is read-only as far as the control and the StyledTextModel base class are concerned; the control's editable property is not relevant
>
> This means that the document cannot be modified via the UI; it cannot be modified by calling control methods from the app (nor by calling the editing methods on StyledTextModel, but apps don't typically do that).
>
> There are a couple of thoughts I have around this.
>
> First, `RichTextArea::canEdit` is not sufficient to distinguish these three cases; it will return false for both B and C. There are methods in RTA that say "if canEdit is false, then this method does nothing". That's correct for case C, but some of those methods -- namely the ones not tied to a UI action (e.g., `appendText`, `insertText`, `replaceText`, `applyStyle`, `setStyle`, `clear`) -- will intentionally do something in case B. Consider adding a second convenience method that returns `model != null && model.isUserEditable` or similar, and use that new method to qualify whether the non-UI mutator methods of RTA do anything.
>
> Second, `StyledTextModel::userEditable` doesn't seem like the right name for the method in the model to convey that it is effectively read-only as far as the control is concerned. Perhaps "writable" would be a better name, since from the point-of-view of the caller of the StyledTextModel, that's what it means. Maybe there is an even better name, but having "user" in the name is misleading (and editable by itself would be too confusing, since that's the name we want to keep for the control, and it doesn't have the same meaning). Or you could call it "readOnly", but then that would flip the sense of the boolean.
Thank you @kevinrushforth . I just wanted to emphasize that unlike any other feature that goes into mainline, this module is an incubating module, with the primary goal of getting the new APIn the hands of the app developers, **in order to receive their feedback**.
This makes the acceptance criteria a bit less stringent, as the incubator may incubate over several releases (or be dropped altogether).
I would really like to target jfx24, even if the code and API has imperfections. And, as always, your feedback and suggestions are greatly appreciated!
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1524#issuecomment-2326881979
More information about the openjfx-dev
mailing list