On Fri, 15 Nov 2024 17:36:25 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> 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, 
>> **jfx.incubator.richtext**. 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**.
>> 
>> Because it's an incubating module, please focus on the public APIs rather 
>> than implementation.  There **will be** changes to the implementation 
>> once/if the module is promoted to the core by popular demand.  The goal of 
>> the incubator is to let the app developers try the new feature out. 
>> 
>> **References**
>> 
>> - Proposal: 
>> https://github.com/andy-goryachev-oracle/Test/blob/main/doc/RichTextArea/RichTextArea.md
>> - Discussion points: 
>> https://github.com/andy-goryachev-oracle/Test/blob/main/doc/RichTextArea/RichTextAreaDiscussion.md
>> - API specification (javadoc): 
>> https://cr.openjdk.org/~angorya/RichTextArea/javadoc
>> - RichTextArea RFE: https://bugs.openjdk.org/browse/JDK-8301121
>> - Behavior doc: 
>> https://github.com/andy-goryachev-oracle/jfx/blob/8301121.RichTextArea/doc-files/behavior/RichTextAreaBehavior.md
>> - CSS Reference: 
>> https://cr.openjdk.org/~angorya/RichTextArea/javadoc/javafx.graphics/javafx/scene/doc-files/cssref.html
>> - InputMap (v3): 
>> https://github.com/andy-goryachev-oracle/Test/blob/main/doc/InputMap/InputMapV3.md
>> - Previous Draft PR: https://github.com/openjdk/jfx/pull/1374
>
> Andy Goryachev has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 63 commits:
> 
>  - whitespace
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - save as
>  - removed function handler
>  - removed add handler last
>  - use focus traversal api
>  - Merge remote-tracking branch 'origin/master' into 8301121.RichTextArea
>  - settings
>  - comment
>  - review comments
>  - ... and 53 more: https://git.openjdk.org/jfx/compare/dd600658...52a7dba1

Here are my first batch of comments on the "InputMap (Incubator)" API.

1. `BehaviorBase`: I'm not yet convinced that this should be part of the public 
API. If it is, it needs a better name; the current name would be suitable for a 
class that all behaviors are required to extend (analogous to SkinBase, which 
skins need to extend, because Control takes a SkinBase). This behavior base 
class is optional and supports `SkinInputMap.Stateful` only.

2. `SkinInputMap.Stateful` / `SkinInputMap.Stateless`: The `.Stateful` and 
`.Stateless` split doesn't seem like the right level of abstraction; in 
particular, it doesn't seem like the `.Stateless` class has been fully fleshed 
out. One thought: Could the `.Stateful` and `.Stateless` classes be unified? 
They don't seem all that different, and it could clean up the API.

3. Can `SkinInputMap` be installed only on the `Skin` and not on the `Control` 
(i.e., not in the `InputMap` itself)? There are `SkinInputMap` methods that are 
now package scope that would need accessors to make this work, but that should 
not be a concern (they are already internal interfaces in the current proposal 
and would remain so). Related to this, should `SkinInputMap` and `BehaviorBase` 
be moved to a separate package, something with "skin" in the name? Both of 
these suggestions would help with separation of concerns. These classes are for 
of developers of custom controls (or custom skins / behaviors for existing 
controls) not app developers using `InputMap`.

SUGGESTION: Given the above, and since most of the concerns raised about the 
InputMap API are also around the Behavior and Skins, I recommend that you 
consider making the Skin / Behavior part of the Input Map incubator module 
non-public in the first version? You could make it public in v2, which would 
give more time to address these and other issues.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1524#issuecomment-2486921662

Reply via email to