search

WordPress / twentytwentytwo

Twenty Twenty-Two, the default WordPress theme that will launch with WordPress 5.9.
406 stars 91 forks source link

Add menu container padding #301

Closed kjellr closed 2 years ago

kjellr commented 2 years ago

Closes #292 (It's not perfect, but it's the best we can do at the moment)

This PR modifies the default padding for the menu container so that it is in sync with with the site's outer padding.

This means that (if you use the default header, have a one-line title, and have no site logo), the close button will probably line up with the menu button. But even without that set of specific circumstances, I think this is a reasonable move so that the outer spacing matches the site.

Before After
space-old space
kjellr commented 2 years ago

This feels pretty good, and makes sense conceptually alongside the outer padding CSS already in the PR. Could use a review when someone has a moment.

jasmussen commented 2 years ago

Ack. I have noticed the course button being a bit twitchy, almost as if it closes onmousedown rather than onmouseup. Something we need to fix at the source.

kjellr commented 2 years ago

Ok, we can hold off on this one for now I think. We can push this back in after the holidays. 👍

chthonic-ds commented 2 years ago

encountering an issue where closing the modal reopens it, since the buttons are now in the same place:

Hopefully fixed by https://github.com/WordPress/gutenberg/pull/36837.

jasmussen commented 2 years ago

Thank you @chthonic-ds, yes, the issue does appear to be fixed by WordPress/gutenberg#36837. I just tested again. You can't quite see when I click, but in this GIF I took great care to click and hold on the burger icon, then release. Same with the close button, where I intentionally positioned the cursor over the adminbar. In the past, clicking the close icon when over the adminbar I'd accidentally go to the admin profile page, because the item underneith also receives the click: test

In other words, this PR should be good to go.

kjellr commented 2 years ago

Yep, I can't reproduce either anymore. I'll merge this in for the next release. Thank you all!