Skip to content

fix(max-menu): add early return in setSubmenuWidth to prevent global state...

Max de Vries requested to merge feature/max-menu-transition-container-ref into master

This fixes a small but noticeable bug where the final top level menu item would cancel out the earlier set submenuWidth for RecursiveMenuMaxMenu in the global state if it doesn't have submenu items, thus causing the hover effects to never take place.

To give it more explanation

Take the following component tree example:

<MenuList>
    ├── MaxMenuMenuItemLinkElement  (Item A: depth 1, has_children) ← ref attached ✓
    │   └── div.submenu [ref]
    │       ├── MaxMenuMenuItemLinkElement  (Child 1: depth 2)      ← ref null
    │       ├── MaxMenuMenuItemLinkElement  (Child 2: depth 2)      ← ref null
    │       └── MaxMenuMenuItemLinkElement  (Child 3: depth 2)      ← ref null
    ├── MaxMenuMenuItemLinkElement  (Item B: depth 1, no children)  ← ref null
    └── MaxMenuMenuItemLinkElement  (Item C: depth 1, no children)  ← ref null

useMenuTransition is called at the top of MaxMenuMenuItemLinkElement — before the if (depth === 1 && has_children) check. So items B and C also call the hook and each get a submenuContainerRef that is never attached to any DOM element. React runs useEffect in tree order (top-down, siblings left-to-right):

Order Component submenuContainerRef.current Width set to
1 Child 1 (depth 2) null 0
2 Child 2 (depth 2) null 0
3 Child 3 (depth 2) null 0
4 Item A (depth 1 w/ children) <div> real width
5 Item B (depth 1, no children) null 0 ← stomps it
6 Item C (depth 1, no children) null 0 ← stomps it

All of them write to the same shared context state (submenuContainerWidth). Item A correctly sets the real width, but items B and C run their effects after A and overwrite it back to 0.

This is now an issue on SWV. But we never saw this on NIOZ, since the last top level item has subitems. If you turn those off in the menu, the same problem occurs.

Merge request reports

Loading