Skip to content

Commit

Permalink
fix: fix setting class name to menu-bar items in overflow (vaadin#6209)
Browse files Browse the repository at this point in the history
* fix: fix setting class name to menu-bar items in overflow

The changes done in vaadin#6031 doesn't work if the root item is inside the
overflow part of the `MenuBar`. That happens because the `setClassName`
method in connector changes the `className` value in the
`component._item` object that should be reflected to the item when the
items are regenerated.

Usually, the `_item` object of `component` refers to one instance inside
of the `__generatedItems` array in the menu bar instance, but when the
menu item becomes part of the overflow, it receives a new instance of
`_item` and the reference is lost.

Saving the original reference in another property solves the issue and
now the class name is changed properly for items inside the overflow.

* test: check for class name changes in overflow item

* refactor: apply review suggestions

* fix: use _item when _rootItem is undefined (submenu items)

* test: add test checking item in and out overflow
  • Loading branch information
DiegoCardoso authored Apr 17, 2024
1 parent b935837 commit ee2443e
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -233,17 +233,38 @@ public MenuBarTestPage() {
});
setUnsetSubItemClassNameButton.setId("set-unset-sub-item-class-name");

NativeButton setItem2ClassNameButton = new NativeButton(
"set item 2 class", e -> {
item2.setClassName(MENU_ITEM_FIRST_CLASS_NAME);
});
setItem2ClassNameButton.setId("set-item2-class-name");

NativeButton removeItem2ClassNameButton = new NativeButton(
"remove item 2 class", e -> {
item2.setClassName(null);
});
removeItem2ClassNameButton.setId("remove-item2-class-name");

NativeButton changeItem2ClassNameButton = new NativeButton(
"change item 2 class", e -> {
item2.setClassName(MENU_ITEM_SECOND_CLASS_NAME);
});
changeItem2ClassNameButton.setId("change-item2-class-name");

add(new Hr(), addRootItemButton, addSubItemButton, removeItemButton,
openOnHoverButton, setWidthButton, resetWidthButton,
disableItemButton, toggleItem1VisibilityButton,
toggleItem2VisibilityButton, checkedButton,
toggleAttachedButton, setI18nButton, toggleAttachedButton,
toggleMenuBarThemeButton, toggleItem1ThemeButton,
toggleSubItemThemeButton, toggleClassNameButton,
setItemClassNameButton, setUnsetClassNameButton,
addRemoveMultipleClassNames, toggleSubItemClassNameButton,
addSecondSubItemClassButton, removeSubItemClassNameButton,
setItemClassNameButton, setItem2ClassNameButton,
removeItem2ClassNameButton, changeItem2ClassNameButton,
setUnsetClassNameButton, addRemoveMultipleClassNames,
toggleSubItemClassNameButton, addSecondSubItemClassButton,
removeSubItemClassNameButton,
addRemoveMultipleSubItemClassNames,
setUnsetSubItemClassNameButton);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,52 @@ public void subMenuItem_classNamesAreToggleWithSet_classNamesAreToggled() {
MenuBarTestPage.SUB_ITEM_FIRST_CLASS_NAME);
}

@Test
public void menuItemWithClassNameInOverflow_changeClassName_classNameIsChanged() {
click("set-width");
click("set-item2-class-name");
waitForResizeObserver();
menuBar.getOverflowButton().click();
click("change-item2-class-name");
menuBar.getOverflowButton().click();
TestBenchElement menuItem = menuBar.getSubMenuItems().get(0);
Assert.assertEquals(menuItem.getAttribute("class"),
MenuBarTestPage.MENU_ITEM_SECOND_CLASS_NAME);
}

@Test
public void menuItemWithClassNameInOverflow_removeClassName_classNameIsRemoved() {
click("set-width");
click("set-item2-class-name");
waitForResizeObserver();
menuBar.getOverflowButton().click();
click("remove-item2-class-name");
menuBar.getOverflowButton().click();
TestBenchElement menuItem = menuBar.getSubMenuItems().get(0);

Assert.assertFalse(menuItem.getAttribute("class")
.contains(MenuBarTestPage.MENU_ITEM_SECOND_CLASS_NAME));
}

@Test
public void menuItemWithClassNameInOverflow_menuItemLeavesOverflow_classNameCanBeChanged() {
click("set-width");
click("set-item2-class-name");
waitForResizeObserver();
menuBar.getOverflowButton().click();
click("reset-width");
waitForResizeObserver();
click("change-item2-class-name");
TestBenchElement menuItem = menuBar.getButtons().get(1);
Assert.assertEquals(menuItem.getAttribute("class"),
MenuBarTestPage.MENU_ITEM_SECOND_CLASS_NAME);

click("remove-item2-class-name");
menuItem = menuBar.getButtons().get(1);
Assert.assertFalse(menuItem.getAttribute("class")
.contains(MenuBarTestPage.MENU_ITEM_SECOND_CLASS_NAME));
}

@Test
public void setMenuItemTheme_toggleVisibility_themeIsPreserved() {
click("toggle-item-1-theme");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,14 @@ import './contextMenuConnector.js';

let items = menubar.__generatedItems || [];

// Propagate disabled state from items to parent buttons
items.forEach((item) => (item.disabled = item.component.disabled));
items.forEach((item) => {
// Propagate disabled state from items to parent buttons
item.disabled = item.component.disabled;

// Saving item to component because `_item` can be reassigned to a new value
// when the component goes to the overflow menu
item.component._rootItem = item;
});

// Observe for hidden and disabled attributes in case they are changed by Flow.
// When a change occurs, the observer will re-generate items on top of the existing tree
Expand Down Expand Up @@ -110,9 +116,11 @@ import './contextMenuConnector.js';
};
}

function setClassName (component) {
if (component._item) {
component._item.className = component.className;
function setClassName(component) {
const item = component._rootItem || component._item;

if (item) {
item.className = component.className;
}
}

Expand Down

0 comments on commit ee2443e

Please sign in to comment.