Skip to content

fix: use path-based menuId to prevent collisions in nested submenus#3762

Open
chrism wants to merge 1 commit intosoftware-mansion:mainfrom
chrism:fix/submenu-menuId-collision
Open

fix: use path-based menuId to prevent collisions in nested submenus#3762
chrism wants to merge 1 commit intosoftware-mansion:mainfrom
chrism:fix/submenu-menuId-collision

Conversation

@chrism
Copy link
Copy Markdown

@chrism chrism commented Mar 14, 2026

Summary

  • prepareMenu passes menuIndex as index when recursing into submenus, losing the original bar button item index
  • This causes menuId collisions between actions in different submenus and sibling toolbar menus at the same position
  • Fix threads a path string through recursion to track nesting depth, while preserving the original bar button item index

Problem

Given two toolbar menu buttons — a filter menu at bar button index 1 containing submenus, and a sort menu at bar button index 2:

Toolbar:
  [+] button (index 0)
  [filter] menu (index 1)     ← contains Location, Item Type, Quality submenus
  [sort] menu (index 2)       ← contains Title, Gear Type, Date Added actions

When prepareMenu recurses into the Quality submenu (at menuIndex 2 inside the filter menu), it calls:

prepareMenu(menuItem, menuIndex, side)  // menuIndex=2, replacing index=1

The first action inside Quality then gets menuId: "0-2-right", which collides with the first action in the sort menu ("0-2-right"). Selecting a quality filter fires the sort handler instead.

Fix

Add a path parameter that tracks position through nesting, while keeping the original bar button index:

const prepareMenu = (
  menu, index, side,
  path = '',  // NEW
) => {
  return {
    ...menu,
    items: menu.items.map((menuItem, menuIndex) => {
      const currentPath = path ? `${path}.${menuIndex}` : `${menuIndex}`;
      // ...
      if (menuItem.type === 'submenu') {
        return {
          ...menuItem,
          ...prepareMenu(menuItem, index, side, currentPath),
          //                       ^^^^^ preserved (was menuIndex)
        };
      }
      return {
        ...menuItem,
        menuId: `${currentPath}-${index}-${side}`,
        //        ^^^^^^^^^^^^ path-based (was menuIndex)
      };
    }),
  };
};

Before: Quality "All" = 0-2-right (collides with Sort "Title" = 0-2-right)
After: Quality "All" = 2.0-1-right, Sort "Title" = 0-2-right (unique)

Verified

  • Toolbar with multiple menu buttons — actions in each menu fire the correct handler
  • Nested submenus (inline sections) within a toolbar menu — no cross-talk with sibling toolbar menus
  • Single menu button with no submenus — no regression (path defaults to menuIndex)

When prepareMenu recurses into submenus, it passes menuIndex as the
index parameter instead of preserving the original bar button item
index. This causes menuId collisions between actions in different
submenus and sibling toolbar menus that share the same position.

For example, with two toolbar menu buttons (filter at index 1, sort
at index 2), a submenu item at menuIndex 2 inside the filter menu
generates menuId "0-2-right", which collides with the first action
in the sort menu ("0-2-right"). This causes menu action callbacks
to fire on the wrong handler.

Fix by threading a path string through recursion that tracks the
nesting position (e.g. "2.0" for the first item in the third
submenu), while preserving the original bar button item index.
This produces unique menuIds like "2.0-1-right" that never collide
with sibling menus.
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.

1 participant