-
Notifications
You must be signed in to change notification settings - Fork 70
refactor: deprecate hascomponents api and provide alternatives #7357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
refactor: deprecate hascomponents api and provide alternatives #7357
Conversation
*/ | ||
@Deprecated(since = "24.8") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that add()
is used internally by addSeparator()
and we probably should refactor that method:
Lines 64 to 66 in 4e414bb
public void addSeparator() { | |
add(new Hr()); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not mistaken, the add/remove methods in SubMenu
are completely separate from the ones in ContextMenuBase
. They are defined in SubMenuBase
, which does not seem to extend ContextMenuBase
or implement HasComponents
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, I missed that. We probably could consider adding addSeparator()
also to ContextMenu
for consistency, there is currently an integration test explicitly covering this use case:
Lines 155 to 158 in 22d9c2b
// Components can also be added to the overlay | |
// without creating menu items with add() | |
Component separator = new Hr(); | |
contextMenu.add(separator, new Label("This is not a menu item")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would that qualify as a feature? Maybe we can leave that for another PR, what do you think?
651f14f
to
b462707
Compare
b462707
to
270cc14
Compare
|
Description
This PR
HasComponents
APIremoveItem
andremoveAllItems
, which work the same way as beforePart of #5532
Type of change
Checklist