Skip to content

Menu should not close when toggling a setting item #8827

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

Closed
Artur- opened this issue Mar 20, 2025 · 3 comments · Fixed by #8829 or #8871
Closed

Menu should not close when toggling a setting item #8827

Artur- opened this issue Mar 20, 2025 · 3 comments · Fixed by #8829 or #8871

Comments

@Artur-
Copy link
Member

Artur- commented Mar 20, 2025

Description

With a custom toggleable item in a menubar, the user should be able to toggle the item and the menu should be updated and also kept open. There does not seem to be a way to accomplish this at the moment.

Expected outcome

The user can open the menu, toggle a setting, the item text is updated and the user can toggle the next setting

Minimal reproducible example

@customElement('my-app')
export class MyApp extends LitElement {
  @state()
  private items: MenuBarItem[] = [];

  private setting1 = false;
  private setting2 = true;
  private setting3 = false;

  connectedCallback(): void {
    super.connectedCallback();
    this.updateItems();
  }

  createMenuItem(text: string, state: boolean): HTMLElement {
    const item = document.createElement('vaadin-menu-bar-item');
    item.innerText = text + '-' + (state ? 'ON' : 'OFF');
    return item;
  }

  updateItems() {
    this.items = [
      {
        text: 'Menu',
        children: [
          {
            text: 'Settings',
            children: [
              {
                component: this.createMenuItem('Setting 1', this.setting1),
              },
              {
                component: this.createMenuItem('Setting 2', this.setting2),
              },
              {
                component: this.createMenuItem('Setting 3', this.setting3),
              },
            ],
          },
        ],
      },
    ];
  }

  render() {
    return html`<vaadin-menu-bar
      @item-selected="${(e: CustomEvent) => {
        this.setting1 = !this.setting1;
        this.updateItems();
      }}"
      .items=${this.items}></vaadin-menu-bar>`;
  }
}

Steps to reproduce

Use the code

Environment

Vaadin version(s): 24.7.0
OS: mac os

Browsers

No response

@web-padawan
Copy link
Member

The menu is closed by this logic which I think was originally introduced to handle cases where items is updated so that the original item which was associated with the opened submenu is no longer present in the DOM:

const subMenu = this._subMenu;
if (subMenu && subMenu.opened) {
subMenu.close();
}

However, even with that logic removed, there would be still a problem caused to the fact that submenu overlay still has positionTarget pointing to the old vaadin-menu-bar-button is no longer in the DOM after buttons are re-rendered. As a result, the following check in the _updatePosition() method would cause the overlay to be closed:

if (targetRect.width === 0 && targetRect.height === 0 && this.opened) {
this.opened = false;
return;
}

We could try to use Lit in __renderButtons() method in the MenuBarMixin nstead of manual DOM manipulations, so that buttons would not get recreated unnecessarily and the positionTarget would still point to the correct button.

@web-padawan
Copy link
Member

Reopening since the PR will be reverted for now as the implementation breaks ITs in Flow counterpart.

@web-padawan
Copy link
Member

Created a PR with a new fix: #8871. Still need to figure out how to test the Flow component issue properly.

@yuriy-fix yuriy-fix added Impact: Low refactor Internal improvement labels Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants