On Fri, 3 May 2024 21:00:26 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Adds **Labeled.textTruncated** property which indicates when the text is >> visually truncated (and the ellipsis string is inserted) in order to fit the >> available width. >> >> The new property is being set by the code which computes the actual text >> string to be displayed (and which inserts the ellipsis string) in >> `LabeledSkinBase.updateDisplayedText(double,double)`. >> >> >> **Alternative** >> >> None exists as this requires changes to the core (Utils). >> >> >> **See Also** >> >> * [JDK-8327483](https://bugs.openjdk.org/browse/JDK-8327483) TreeView: Allow >> for tooltip when cell text is truncated >> * [JDK-8205211](https://bugs.openjdk.org/browse/JDK-8205211) Ability to show >> Tooltip only when text is shown with ellipsis (...) > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last revision: > > whitespace Yes, this looks like a better approach for the implementation. I did a quick test and it now works as expected. I left a couple comments inline. modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/Utils.java line 221: > 219: OverrunStyle type, > 220: String ellipsisString, > 221: AtomicBoolean textTruncated I recommend setting `textTruncated` to false as the first statement (alternatively, add a comment that the caller is expected to initialize it to false). modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/Utils.java line 436: > 434: OverrunStyle truncationStyle, > 435: String ellipsisString, > 436: AtomicBoolean textTruncated, Similarly, I recommend setting it to false as the first statement. In this case, I see that it will be set to false in the case where it makes it to the return statement at the end of the method without being truncated, but there is one early return where this isn't the case. modules/javafx.controls/src/main/java/javafx/scene/control/Labeled.java line 855: > 853: } > 854: > 855: private ReadOnlyBooleanWrapper textTruncated() { Suggestion: rename this method to something like `textTruncatedImpl()` for clarity (as it is, the method, which is a writable property, shares the same name as the read-only property field, which could be confusing). ------------- PR Review: https://git.openjdk.org/jfx/pull/1389#pullrequestreview-2039358463 PR Review Comment: https://git.openjdk.org/jfx/pull/1389#discussion_r1589993165 PR Review Comment: https://git.openjdk.org/jfx/pull/1389#discussion_r1589993485 PR Review Comment: https://git.openjdk.org/jfx/pull/1389#discussion_r1589994760
