On Mon, 24 Nov 2025 21:44:59 GMT, Andy Goryachev <[email protected]> wrote:
>> Did you read what I wrote above? I made multiple suggestions as how we can >> deal with the situation, even without making things managed again. >> >> To reiterate, what is the problem? >> - We now have a dependency from `TextCell` to `VFlow`. Not explicitly, but >> rather hidden, by searching all parents (which is also costly when doing >> that often). I would prefer a clear separation of concerns >> - This makes subclassing/extending harder. What happens, if I want to write >> my own `VFlow`, but still use everything else, including `TextCell`? What if >> I want to use `TextCell` for a component, that has no `VFlow`? What happens >> if we find a `VFlow` from another component even? Because we used the >> `TextCell` somewhere else? >> - Other components made it clear how to use `managed` and `unamanged` nodes. >> And how to bubble up a request still. Why do we want to make an exception >> here? This is the first time we need to search the parent hierarchy >> - Why not e.g. binding to the `needsLayoutProperty` from a `TextCell` from >> within `VFlow` and just request the layout when set? >> >> Also another point: The test is green for me with the changes from: >> https://github.com/openjdk/jfx/pull/1945 >> So I would like to know, if maybe there was a bug even? > > Thank you for looking into this and offering your suggestions! The thing is, > the RTA is a bit more complex than the situation you describe, due to the > presence of other requirements (such as side areas that house line numbers > and possibly other paragraph decorators). > > The design of the skin is such that only `VFlow` does the layout. The VFlow > is the sole actor that deals with its complicated content. Without a very > good reason, this design is unlikely to change. Furthermore, the `VFlow` is > still an implementation detail, so one can't just subclass the skin (this is > true for _other controls as well_). Neither `VFlow` nor TextCell nor > `RichUtils` are public API. > > Yes, the test is green with #1945 but it fails in master. And yes, the bug > is still there even with #1945 , as can be seen with the monkey tester: > > expected: > <img width="565" height="333" alt="Screenshot 2025-11-24 at 13 27 15" > src="https://github.com/user-attachments/assets/67c155e4-ca21-432a-91c7-3ea96e0dce50"; > /> > > observed: > <img width="569" height="259" alt="Screenshot 2025-11-24 at 13 27 05" > src="https://github.com/user-attachments/assets/433a96a5-9c9d-422a-b845-d14e8fb4c5b3"; > /> While looking at the test, discovered an issue with the `SimpleViewOnlyStyledModel`: https://bugs.openjdk.org/browse/JDK-8372438 thanks! ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1975#discussion_r2557815520
