-
Notifications
You must be signed in to change notification settings - Fork 437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8301121: RichTextArea Control (Incubator) #1374
base: master
Are you sure you want to change the base?
8301121: RichTextArea Control (Incubator) #1374
Conversation
👋 Welcome back angorya! A progress list of the required criteria for merging this PR into |
82e203a
to
425b5f4
Compare
8cf56a4
to
fa51991
Compare
❗ This change is not yet ready to be integrated. |
@andy-goryachev-oracle this pull request can not be integrated into git checkout 8301121.rich.text.area.incubator
git fetch https://git.openjdk.org/jfx.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
I tried the manual test applications and have few observations:
CodeAreaDemoApp:
In RichEditorDemoApp, bold, underline and italic buttons did not work when the app was launched for the first time. After relaunching the app, only italic button is not working. |
Thank you @karthikpandelu for testing!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started taking a look. A few general questions -
-
What level of CSS support does the RichTextArea control have?
For e.g. TextArea has this level of support -
https://docs.oracle.com/javafx/2/api/javafx/scene/doc-files/cssref.html#textarea -
How is 2B row limit imposed?
-
How is the performance impact as and when model size increases? As it is a sliding window design, I believe, it should become constant at some point of model size.
public static final StyleAttribute<Double> LINE_SPACING = new StyleAttribute<>("LINE_SPACING", Double.class, true); | ||
|
||
/** Right-to-left orientation paragraph attribute */ | ||
public static final StyleAttribute<Boolean> RIGHT_TO_LEFT = new StyleAttribute<>("RIGHT_TO_LEFT", Boolean.class, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be ORIENTATION and it's value either be RIGHT_TO_LEFT or LEFT_TO_RIGHT (default)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good question. Originally, it was intended as a simple boolean flag, but really it might have 3 states: LTR, RTL, INHERIT (or null), as a way to explicitly override the paragraph orientation.
We still lack proper support for RTL/mixed text, so perhaps I ought to make this attribute private until we get RTL fixed.
|
||
/** | ||
* An immutable object containing style attributes. | ||
* TODO name: StyleSet? (though it's not a set) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some attributes listed under StyleAttrs
are related to Paragraph
and some are related to text
(like font size etc). We can segregate them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would you suggest?
Place in different classes, re-arrange in two blocks, or something else?
Thank you @aghaisas for taking a look and asking these questions!
A good question. At the moment,
While
We could add more. The question is - does it make sense to make all the trivial properties styleable? What is the general rule that we should use to make properties styleable?
Paragraph index is an
Correct - for most operations, the performance is a function of the text being laid out in the sliding window. Obviously, very long paragraphs will slow things down. (There is currently a bug where editing the Writing Systems model is slow, I'll be working on that shortly). Of course, exporting large chunks of text from a very large model will take time proportional to the size of the text being exported. Copying a large selection might throw an OOM Error. One thing was explicitly added to the layout logic: when |
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, javafx.incubator.controls. 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.
References
[0] Proposal: https://github.com/andy-goryachev-oracle/Test/blob/rich.jep.review/doc/RichTextArea/RichTextArea.md
[1] Discussion points: https://github.com/andy-goryachev-oracle/Test/blob/rich.jep.review/doc/RichTextArea/RichTextAreaDiscussion.md
[2] API specification (javadoc): https://cr.openjdk.org/~angorya/RichTextArea/javadoc
[3] Draft Pull Request for API comments and feedback: #1374
[4] RichTextArea RFE: https://bugs.openjdk.org/browse/JDK-8301121
[5] Missing APIs related to rich text control: https://bugs.openjdk.org/browse/JDK-8300569
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1374/head:pull/1374
$ git checkout pull/1374
Update a local copy of the PR:
$ git checkout pull/1374
$ git pull https://git.openjdk.org/jfx.git pull/1374/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1374
View PR using the GUI difftool:
$ git pr show -t 1374
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1374.diff