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.

Sign up for GitHub

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
Open

refactor: deprecate hascomponents api and provide alternatives #7357

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 98 additions & 21 deletions ...ontext-menu-flow/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.vaadin.flow.component.contextmenu;

import java.util.Collection;
import java.util.List;
import java.util.stream.Stream;

Expand Down Expand Up @@ -203,17 +204,10 @@ public void close() {
/**
* Adds a new item component with the given text content to the context menu
* overlay.
* <p>
* This is a convenience method for the use case where you have a list of
* highlightable {@link MenuItem}s inside the overlay. If you want to
* configure the contents of the overlay without wrapping them inside
* {@link MenuItem}s, or if you just want to add some non-highlightable
* components between the items, use the {@link #add(Component...)} method.
*
* @param text
* the text content for the created menu item
* @return the created menu item
* @see #add(Component...)
*/
public I addItem(String text) {
return getMenuManager().addItem(text);
Expand All @@ -222,17 +216,10 @@ public I addItem(String text) {
/**
* Adds a new item component with the given component to the context menu
* overlay.
* <p>
* This is a convenience method for the use case where you have a list of
* highlightable {@link MenuItem}s inside the overlay. If you want to
* configure the contents of the overlay without wrapping them inside
* {@link MenuItem}s, or if you just want to add some non-highlightable
* components between the items, use the {@link #add(Component...)} method.
*
* @param component
* the component to add to the created menu item
* @return the created menu item
* @see #add(Component...)
*/
public I addItem(Component component) {
return getMenuManager().addItem(component);
Expand All @@ -241,10 +228,6 @@ public I addItem(Component component) {
/**
* Adds the given components into the context menu overlay.
* <p>
* For the common use case of having a list of high-lightable items inside
* the overlay, you can use the {@link #addItem(String)} convenience methods
* instead.
* <p>
* The added elements in the DOM will not be children of the
* {@code <vaadin-context-menu>} element, but will be inserted into an
* overlay that is attached into the {@code <body>}.
Expand All @@ -253,23 +236,98 @@ public I addItem(Component component) {
* the components to add
* @see HasMenuItems#addItem(String, ComponentEventListener)
* @see HasMenuItems#addItem(Component, ComponentEventListener)
*
* @deprecated Since 24.8, use {@link #addItem(Component)} instead
*/
@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:

flow-components/vaadin-context-menu-flow-parent/vaadin-context-menu-flow/src/main/java/com/vaadin/flow/component/contextmenu/SubMenu.java

Lines 64 to 66 in 4e414bb

public void addSeparator() {
add(new Hr());
}

Copy link
Contributor Author

@ugur-vaadin ugur-vaadin Apr 29, 2025 •
edited
Loading

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:

flow-components/vaadin-context-menu-flow-parent/vaadin-context-menu-flow-integration-tests/src/main/java/com/vaadin/flow/component/contextmenu/it/ContextMenuView.java

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

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?

@Override
public void add(Component... components) {
getMenuManager().add(components);
}

/**
* @inheritDoc
*
* @deprecated Since 24.8, use {@link #addItem(Component)} instead
*/
@Deprecated(since = "24.8")
@Override
public void add(Collection<Component> components) {
HasComponents.super.add(components);
}

/**
* @inheritDoc
*
* @deprecated Since 24.8, use {@link #addItem(String)} instead
*/
@Deprecated(since = "24.8")
@Override
public void add(String text) {
HasComponents.super.add(text);
}

/**
* @inheritDoc
*
* @deprecated Since 24.8, use {@link #addItem(Component)} instead
*/
@Deprecated(since = "24.8")
@Override
public void addComponentAsFirst(Component component) {
HasComponents.super.addComponentAsFirst(component);
}

/**
* @inheritDoc
*
* @deprecated Since 24.8, use {@link #removeItem(Component...)} instead
*/
@Deprecated(since = "24.8")
@Override
public void remove(Component... components) {
removeItem(components);
}

/**
* Removes the provided components from the context menu overlay.
*
* @param components
* the components to remove
*/
public void removeItem(Component... components) {
getMenuManager().remove(components);
}

/**
* Removes all of the child components. This also removes all the items
* added with {@link #addItem(String)} and its overload methods.
* @inheritDoc
*
* @deprecated Since 24.8, use {@link #removeItem(Component...)} instead
*/
@Deprecated(since = "24.8")
@Override
public void remove(Collection<Component> components) {
removeItem(components == null ? null
: components.toArray(new Component[0]));
}

/**
* Removes all the child components. This also removes all the items added
* with {@link #addItem(String)} and its overload methods.
*
* @deprecated Since 24.8, use {@link #removeAllItems()} instead
*/
@Deprecated(since = "24.8")
@Override
public void removeAll() {
removeAllItems();
}

/**
* Removes all the child components. This also removes all the items added
* with {@link #addItem(String)} and its overload methods.
*/
public void removeAllItems() {
getMenuManager().removeAll();
}

Expand All @@ -284,10 +342,29 @@ public void removeAll() {
* the index, where the component will be added
* @param component
* the component to add
* @see #add(Component...)
*
* @deprecated Since 24.8, use {@link #addItemAtIndex(int, Component)}
* instead
*/
@Deprecated(since = "24.8")
@Override
public void addComponentAtIndex(int index, Component component) {
addItemAtIndex(index, component);
}

/**
* Adds the given component into this context menu at the given index.
* <p>
* The added elements in the DOM will not be children of the
* {@code <vaadin-context-menu>} element, but will be inserted into an
* overlay that is attached into the {@code <body>}.
*
* @param index
* the index, where the component will be added
* @param component
* the component to add
*/
public void addItemAtIndex(int index, Component component) {
getMenuManager().addComponentAtIndex(index, component);
}

Expand Down