On Thu, 9 Jul 2026 21:46:46 GMT, Andy Goryachev <[email protected]> wrote:
>> modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/FileListFormatHandler.java
>> line 91:
>>
>>> 89: * @param files the list of files to be inserted
>>> 90: */
>>> 91: public static void handleDrop(RichTextArea t, TextPos p, List<File>
>>> files) {
>>
>> Who calls this static method?
>
> RichEditorDemoWindow:118
>
> this is a convenience method for DnD. I think this class is the right place
> to expose this functionality, but I am open to suggestions.
I see what this is doing now. This is not the right place to put this
convenience method. It is in a class that otherwise isn't accessed by
applications. Further, it is a static method that does not operate on a
DataFormatHandler at all; it operates on a RichTextArea not on the model, so I
don't see why it would even be anywhere in the model package.
The method also is not named well. The fact that an application can use it to
handle a drop operation doesn't describe what it does.
How about moving this static helper method to be an instance method in
RichTextArea and renaming it something like `insertImages` or
`insertImageFiles` or ... ?
>> modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/RichTextFormatHandler.java
>> line 198:
>>
>>> 196:
>>> 197: @Override
>>> 198: public StyledInput createStyledInput(Object input,
>>> StyleAttributeMap attr) {
>>
>> Check to see whether this inherits the `@since 27`
>
> does not:
>
> <img width="401" height="442" alt="Image"
> src="https://github.com/user-attachments/assets/cee567d1-d1f6-42fa-8671-01ab59488761";
> />
OK. Looking closer this is good, since the overridden method points to the
parent method, and it is that parent method that was added in 27. No changes
needed.
>> modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledSegment.java
>> line 291:
>>
>>> 289: * Creates a StyledSegment which consists of a single inline Node.
>>> 290: * @param generator the code to create a Node instance
>>> 291: * @param a the segment styles, can be null
>>
>> What does "null" do? Is it different from an empty style attribute map?
>
> yes: `getStyleAttributeMap()` is allowed to return `null`.
OK, but that still doesn't tell me what null does and how it is different from
an empty list. That's really a larger issue though, so OK to defer.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2196#discussion_r3559011034
PR Review Comment: https://git.openjdk.org/jfx/pull/2196#discussion_r3559017624
PR Review Comment: https://git.openjdk.org/jfx/pull/2196#discussion_r3559031117
