Open roblevintennis opened 2 years ago
@vitale232 lmk if you want to take any of this on 🙏
Btw, you may have noticed that the focus outline ring is hidden on the bottom (it's even in some screenshots I put here). Note that this is fixed in another topic branch for the kebab/hamburger/meatball menues so that will be fixed soon'ish.
I just merged latest into my topic branch menu-kebab-hamburger-meatball
and I'm seeing this error in the console (it may be on master
I didn't notice before though so not 100% sure. The keyboard navigation appears to still work it's just throwing the error:
UPDATE: This was an easy fix. In fact, this commit should provide a glimpse into the new menu trigger direction I'm going which might be helpful for the general menu trigger refactoring we'll need to do for Angular Menu component.
These are some follow ups for us to do from first iteration of the angular Menu component implementation:
The default menu trigger button (same we've been using basically) is now being called the 'simple' variant and is the default. The new variants are called 'kebab', 'hamburger' and so on. Passing
type: 'kebab'
into the main Menu component will need to result in a Kebab menu trigger button variant. Not sure how this will best be implemented in Angular but in Vue, we teased out aMenuTrigger
component which can produce one of the variants'simple' | 'kebab' | 'hamburger' | 'meatball'
.Here's a screenshot from the CSS package to visually show the new variants:
Implement
onTriggerKeydown
(or some other implementation detail) and just listen for case when you pressArrowDown
and the menu options are closed; this should result in the menu being opened. See the docs page examples and try putting focus and hitting arrow down to see what I mean.I'm seeing that in this story spec verifying
closeOnSelect
set tofalse
, we're also for some reason NOT closing on click outside. But that case is supposed to happen in theNo Close On Select Or Click Outside
story which sets bothcloseOnSelect
andcloseOnClickOutside
tofalse
. Put differently, for this spec we should only NOT close on clicking the menu option. This is the story I'm talking about:[isSelected]
callback there to fix. This is helpful to be able to ensure the menu selected is remembered when I interact with other menues (I interacted with the second and third menues besides this one and wanted to be able to confirm the first menu selection wasn't inadvertently forgotten or something). Maybe visual paints what I'm trying to do better (notice I see the selections for the 2nd and 3rd menues but not the 1st)~:Update: The decision was made to make the whole
[isSelected]
optional for cases like action menues where it's a sort of “select and go” on a local branch I've added the following (so this can be considered DONE):5e2f41c4