Skip to content
This repository has been archived by the owner on Dec 1, 2019. It is now read-only.

Update the header and menu modal with the conditionals for the new header structure #282

Conversation

andersnoren
Copy link
Contributor

This PR builds on the menu location updates @carolinan made in #247, and updates it with the conditionals and fallbacks specified in #246. It also updates the style to match the design.

I couldn't find a way to replicate the wp_nav_menu sub menu button toggles and chevron icons in wp_list_pages without including a custom walker that modifies start_el. If we can find a way to do this without using a custom walker, that would be much, much better.

In addition to the things in that issue, it includes:

@pattonwebz
Copy link
Member

I am seeing a little strangeness on the admin bar when search/menu appears.

Peek 2019-09-15 14-54

Also a couple clicks on menu ends up putting it into an open but not visible state for some reason.

@andersnoren
Copy link
Contributor Author

andersnoren commented Sep 15, 2019

@andersnoren I've updated the modal styles to include a top offset equal to the height of the .admin-bar, but it won't work until the JavaScript is updated with a different scroll lock method. Since the header toggles are always at the top of the screen, we can simply scroll to the top and set the body to overflow: hidden when a modal is shown, at which point the modals will be positioned correctly.

As for the toggles, yes, the JavaScript handling that should definitely be rewritten as well :)

@pattonwebz
Copy link
Member

@andersnoren did you mean to mention yourself or were you trying to mention me? lol I am going to look a bit at javascript stuff now so will try see if we can resolve some of that before end of day :)

@andersnoren
Copy link
Contributor Author

@pattonwebz A bit quick there 😛 Awesome!

@joyously
Copy link

I couldn't find a way to replicate the wp_nav_menu sub menu button toggles and chevron icons in wp_list_pages without including a custom walker that modifies start_el.

I've had this problem before and I'm sure a lot of theme authors have this problem. Could we make a patch for core with a filter or something in the right place so we don't all have to write custom walkers?

@pattonwebz
Copy link
Member

So I tested the javascript PR at #163 and it does seem to be working. I was unable to properly test merging it with this PR but I think there are some conflicts we need to spend time tomorrow working though so we can get both this and the javascript swap PR into master

@andersnoren andersnoren merged commit 1cd704a into WordPress:master Sep 16, 2019
@andersnoren andersnoren deleted the updates/modify-conditionals-in-modal-menu branch September 16, 2019 17:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Icon-only controls are not accessible A11y: display close button in the menu panel
3 participants