[ContextMenu][DropdownMenu] Fix issue introduced by new selector prop

[benoitgrelard]

Closes #402

After checking a reported issue #402, I realised we have introduced a bug in dependents of Menu (ContextMenu, DropdownMenu) when we added the selector prop.

This is because the expectation on what attributes are present changed, and we didn't account for it here.
Namely, before we were expecting the data-radix-menu-item to be present even in dependents of Menu.
This is no longer a safe assumption to make obviously, therefore we cannot rely on this selector.

Looking at other places we build selectors like this (roving focus behaviour, or typeahead behaviour) they both add their own data attribute to ensure they can safely select items. So it seems in this case we should be doing the same somehow. It feels a bit more annoying given that we know there is already a default selectors for items… but obviously now specific to each dependent of Menu).

I haven't made any change yet, wondering what your thoughts were.

[jjenzz]

Ah, oops. It would be good to have a call about this one because I've had a look at the code and wondering if we could get rid of ENABLED_ITEM_SELECTOR altogether.

I commented out the following code and struggling to decipher the difference so would be good to run through it with you:

// focus the menu if the mouse is moved over anything else than an item
onMouseMove={composeEventHandlers(menuProps.onMouseMove, (event) => {
  if (!isItemOrInsideItem(event.target)) menuRef.current?.focus();
})}

Also, the onKeyDown handler could probably use a new itemsRef.current array that each MenuItem would append to in the context.

React.useEffect(() => {
  context.itemsRef.current = [...context.itemsRef.current, ref.current];
}, [context.itemsRef, ref]);

If all else fails, adding a new stable selector for this makes sense to me 🙂

[benoitgrelard]

Always nice to be able to remove unused code, well spotted @jjenzz!