8375444: MenuButton - textTruncated Property is always false#2040
8375444: MenuButton - textTruncated Property is always false#2040FlorianKirmaier wants to merge 4 commits intoopenjdk:masterfrom
Conversation
Fix textTruncated property for MenuButton
|
👋 Welcome back fkirmaier! A progress list of the required criteria for merging this PR into |
|
@FlorianKirmaier This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 30 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
| labeled.textProperty().addListener(shuttler); | ||
|
|
||
| labeledImpl.textTruncatedProperty().subscribe(v -> { | ||
| LabeledHelper.setTextTruncated(labeled, v); |
There was a problem hiding this comment.
should the property be bound instead?
There was a problem hiding this comment.
This is going through the Accessor pattern of Labeled.
I could extend it, to return the BooleanProperty and use it.
Should i?
There was a problem hiding this comment.
oh, right, I forgot it's a read-only property (and I should know).
let me take a look, at first glance it looks right.
|
/reviewers 2 |
|
@andy-goryachev-oracle |
andy-goryachev-oracle
left a comment
andy-goryachev-oracle
left a comment
There was a problem hiding this comment.
Behaves as expected, the implementation looks good (thanks to Subscription).
Minor comments, will re-approve if you make changes.
For reviewers: added "text truncated" indicator to all controls that extend Labeled, grab the latest here:
https://github.com/andy-goryachev-oracle/MonkeyTest
Small code formatting fixes
|
Thank you for the quick feedback! I did the small formatting changes. |
Removed unnecessary newline
Removed commented out print statements. Fixed import order.
|
Thank you for the two quick code reviews! |
|
@FlorianKirmaier this is ready to integrate |
|
/integrate |
|
Going to push as commit bf93e0d.
Your commit was automatically rebased without conflicts. |
|
@FlorianKirmaier Pushed as commit bf93e0d. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
3 participants
For MenuButton and SplitMenuButton the textTruncated Property is always false - but the text is correctly rendered truncated if it is to long.
This PR fixes it, by extending the logic in LabeledImpl - to always forward this property.
I've also extended the existing tests for the truncated property.
It would be great, to get some feedback on how i fixes the bug.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2040/head:pull/2040$ git checkout pull/2040Update a local copy of the PR:
$ git checkout pull/2040$ git pull https://git.openjdk.org/jfx.git pull/2040/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2040View PR using the GUI difftool:
$ git pr show -t 2040Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2040.diff
Using Webrev
Link to Webrev Comment