On Fri, 15 Mar 2024 09:41:25 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> Thank you for the feedback! >> >> The property is being added to Labeled, which happens to be a base class for >> the cells' skins. The truncated property therefore must work in all the >> subclasses, even those where computePrefWidth is hacked via >> `Properties.DEFER_TO_PARENT_PREF_WIDTH`. >> >>> Second, I would rather want to see if this can be changed without adding >>> some flags which once again will not help application developers that may >>> want to extend something like this. >> >> This is a much more difficult task: First, it needs a careful discussion on >> which APIs to add, if any (there is a possibility that some people might say >> that no new APIs are needed as one can always create an instance of Text or >> Label and use it to get the sizing). There might be a need to undo the >> hacks made by the founding father such as >> `Properties.DEFER_TO_PARENT_PREF_WIDTH`. I would say all this is out of >> scope for this PR, as it achieves its goal without introducing the new APIs. >> >>> IMO, the separation of concern is wrong. A Cell should always give back the >>> prefWidth, other callers should think if they want to use the pref width or >>> the table column width. >> >> Why cells are controls is slightly beyond me, but who am I to ask? I would >> say there should be one control in this picture, and that is TableView (or >> any other cell-based control) whose job is to size and lay out its various >> parts. The current design is different, and it is out of scope for this PR >> to change the design. >> >>> In order to move this forward, I would like to see only the necessary >>> changes >> >> And that 's exactly what you see here - only the necessary changes (that >> unfortunately include undoing the earlier hacks in cells). >> >> As for tests - I certainly agree, but that would be a manual test. The >> behavior can be verified with the examples in JDK-8205211, I just felt the >> value of a manual test is rather low. What I would rather see is an >> automated test that actually verifies the new property behavior down to a >> pixel. Any ideas how to make one? > > Okay. > > Tests: Well, there are some properties we can test here: > - wrapText = true -> height should be used -> check truncated > - width changed -> check truncated > - text changed -> check truncated You can check here how I manipulated the size of the stage of the `StageLoader`. https://github.com/openjdk/jfx/pull/1396 Than we can check and set some properties on any `Labeled`, probably the `Label` is good here. And on the stage if needed (width, height). ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1389#discussion_r1526010108
