Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ugur-vaadin
Copy link
Contributor

Description

This PR

  • deprecates all HasComponents API
  • provides alternatives for removing items: removeItem and removeAllItems, which work the same way as before
  • updates JavaDocs based on the deprecations

Part of #5532

Type of change

  • Bugfix
  • Feature
  • Refactor

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/contributing/overview
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

*/
@Deprecated(since = "24.8")
Copy link
Member

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:

Copy link
Contributor Author

@ugur-vaadin ugur-vaadin Apr 29, 2025

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.

Copy link
Member

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:

// 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"));

Copy link
Contributor Author

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?

@ugur-vaadin ugur-vaadin force-pushed the refactor-deprecate-hascomponents-api-and-provide-alternatives branch from 651f14f to b462707 Compare April 30, 2025 06:08
@ugur-vaadin ugur-vaadin force-pushed the refactor-deprecate-hascomponents-api-and-provide-alternatives branch from b462707 to 270cc14 Compare April 30, 2025 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants