fix(Toolbar): do not close submenu and navigate next item when press Left/Right arrow keys

[sophieH29]

fixes #1064

Menu/Toolbar Left and Right arrow should not activate prev/next parent when the focus is in the toolbar submenu. This is expected only in Menu, not in the toolbar

[codecov]

Codecov Report

Merging #1199 into master will increase coverage by <.01%.
The diff coverage is 54.05%.

@@            Coverage Diff             @@
##           master    #1199      +/-   ##
==========================================
+ Coverage   83.22%   83.22%   +<.01%     
==========================================
  Files         752      752              
  Lines        8887     8905      +18     
  Branches     1192     1262      +70     
==========================================
+ Hits         7396     7411      +15     
- Misses       1477     1480       +3     
  Partials       14       14

Impacted Files	Coverage Δ
...ibility/Behaviors/Toolbar/toolbarButtonBehavior.ts	100% <100%> (ø)	⬆️
...ages/react/test/specs/behaviors/testDefinitions.ts	95.57% <100%> (+0.22%)	⬆️
packages/react/src/components/Menu/MenuItem.tsx	58.33% <26.08%> (-1.15%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 688ae67...42f7724. Read the comment docs.

[miroslavstastny]

Technically, it works.

I don't like it much from architecture POV:
Every time I hit left/right arrow, acc behavior calls doNotNavigateNextParentItem and it MenuItem what decides whether to stop propagation of the event based on its submenu state.

If I consider accessibility a separate encapsulated package, this does not work. How should I, as a component developer, know when not to navigate? We can rename the handler to something like doNotNavigateNextParentItemIfYourSubmenuIsOpen but not sure it makes the API contract much better.

[sophieH29]

Every time I hit left/right arrow, acc behavior calls doNotNavigateNextParentItem and it MenuItem what decides whether to stop propagation of the event based on its submenu state.

Thank you for the comment.
doNotNavigateNextParentItem is only called if specified in behavior. Currently specified in toolbarButtonBehaavior. Basically, you say "I don't want to navigate next parent item when I hit right/left arrow on submenu toolbar button"
Agree, that it looks messy and in an unnatural way. Would be happy to hear any ideas on it.

Also, what can be changed is to move check if submenu is opened to behavior

wrapper: {
doNotNavigateNextParentItem: props.menu && props.menuOpen && [
            { keyCode: keyboardKey.ArrowRight },
            { keyCode: keyboardKey.ArrowLeft },
          ]
}

[miroslavstastny]

Also, what can be changed is to move check if submenu is opened to behavior

wrapper: {
doNotNavigateNextParentItem: props.menu && props.menuOpen && [
            { keyCode: keyboardKey.ArrowRight },
            { keyCode: keyboardKey.ArrowLeft },
          ]
}

If it is possible, that would be better.

But even the fact that item behavior knows the implementation details of the components (that item component must do something (stopPropagation) not to break container component) is strange.
But I don't have any better proposal right now, sorry :-/