On Thu, 20 Nov 2025 17:25:26 GMT, Andy Goryachev <[email protected]> wrote:

>> Umm, I find this code ambiguous - so just trying to clarify which of the two 
>> options presented you intend ?
>> If it's the first then maybe an alternative variable/flag name should be 
>> considered ?
>> If it's the second then embedsNode is always set true as soon as any content 
>> is added which means the first option is what is actually occurring ?
>
> The expectation is that most text cells contain `TextFlow`, with the `VFlow` 
> managing the layout.  Once `VFlow` determines the target width, it queries 
> the `TextFlow` for its preferred height to determine the actual size.
> 
> For paragraphs that contains `Regions` - either inline or as whole paragraphs 
> - this logic needs to be augmented to propagate the `requestLayout()` flag up 
> the hierarchy.  We don't want to do this for pure text cells, but we must do 
> it for embedded nodes.
> 
> `addNode(Node)` handles the inline node case, 
> `RichParagraph.of(Supplier<Region>)` does the "full-width" case.
> 
> so to answer your question - if I understand the issue - the `embedsNode` 
> flag was added to enable telling `VFlow` that it needs to reflow because some 
> embedded node asked for it.  Pure text cells don't need this signaling 
> enabled.
> 
> Did I answer your question (satisfactorily :-) ?

I think so ....
> Pure text cells don't need this signaling enabled.

If this is what is intended, then from how I read the code that is not what is 
happening because `add(Node)` is called even for Text only cells. See: 

https://github.com/openjdk/jfx/blob/f87448ec156608527d77a4204e98e08052ffecd1/modules/jfx.incubator.richtext/src/main/java/com/sun/jfx/incubator/scene/control/richtext/VFlow.java#L827-L828

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1975#discussion_r2547084649

Reply via email to