[#765] Enable dropdowns on desktop for all sections #36
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updates the markup so that dropdowns are initialized properly on desktop, just like on mobile. Still needs some work - the menu is setup properly on first load, but after switching to mobile view and back, the menu doesn't get re-initialized properly.
Ensures that the subnavs get the correct values for "visible" and "toggleable" after switching viewports.
The underline around the current menu item is being positioned absolutely with z-index of 10, so it's important to cover it and avoid confusion.
Dropdowns have a different relative parent than the currently-open submenu, so they need magin and padding added to match the expected positioning.
Fixes the behavior of the dropdown on desktop so that only one sub-men can be open at a time.
MiguelAxcar
approved these changes
Mar 10, 2023
There was a problem hiding this comment.
Very good work!
Despite needing some final styling, the functionality is working as expected, and the navigation through keyboard follows a good logic:
- if all dropdowns are closed: parent items + carets
- when user open any dropdown: parent item, caret, submenu items
If user closes the submenu, it restores the navigation.
goldenapples
commented
Mar 28, 2023
| /** | ||
| * Adds class if there's dropdown menu. | ||
| */ | ||
| if ( dropdownMenuItems.lenght !== 0 ) { |
There was a problem hiding this comment.
Suggested change
| if ( dropdownMenuItems.lenght !== 0 ) { | |
| if ( dropdownMenuItems.length !== 0 ) { |
|
@goldenapples - Looks like this is on hold - correct? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates the markup so that dropdown menus are initialized properly on desktop, just like on mobile. Ready for code review, but I also want to get some user testing done, since this design isn't at all obvious in the context of drop downs.
These dropdowns work on click or keyboard, but not on hover, and actually as it stands the designs don't really allow for hover interaction without substantial rework.