Blazored / Menu

A JavaScript free menu library for Blazor and Razor Components applications.
https://blazored.github.io/Menu
MIT License
176 stars 32 forks source link

Add support for keyboard navigation for submenu item #12

Closed peterblazejewicz closed 4 years ago

peterblazejewicz commented 4 years ago

This changes minor details in the BlazoredSubMenu implemenation to support:

~~As the support for event propagation in Blazor is not yet there, the implementation marks support for SPACE keyboard as TODO item (supporting SPACE from code requires cancelling propagation of the event in the handler).~~

The changes do not impact samples details.

Thanks!

blazor-menu

chrissainty commented 4 years ago

Thanks for doing this @peterblazejewicz. I don't know if you've seen, but .NET Core Preview 2 is out and Blazor now supports stopping event propagation. Would you like to make the changes you suggested before I merge this?

peterblazejewicz commented 4 years ago

Would you like to make the changes you suggested before I merge this?

Yes, let me review the docs. The best, if we could use the focused trap only for Space and Enter (I believe they discussed this as part of discussion on how to implement these new apis). Will update and ping back, Thanks!

peterblazejewicz commented 4 years ago

@chrissainty So I've amended the PR, but limited changes to removing references to the event bubbling implementation details and changing to a simple solution, that seems to work for most use cases. I think one should not use the current, opaque 'preventDefault' attributes, as this is not something desired here with the specific key handler. For example: I'd like to prevent default event when Spacebar has been pressed (as there is a scroll event associated with space bar being down) and catch the up to toggle the menu. I cannot just prevent all events, as this breaks supporting events like built-in browser shortcuts while there is a focus on the menu. Let's revisit the details in the future. Thanks!

chrissainty commented 4 years ago

This is great, thanks @peterblazejewicz.

I haven't had a chance to test out the new event management bits in Preview 2 yet. But it sounds like they are a bit all or nothing which might not be great.