Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try: Keep the drawer out of the header area in Interface/FSE #27595

Closed

Conversation

stokesman
Copy link
Contributor

@stokesman stokesman commented Dec 8, 2020

This PR makes the Navigation Panel in the Site Editor open below the header. Doing so allows the header to remain the same width and so allows removal of the fastidious CSS to adjust for that. It also makes it more consistent with how the sidebars and inserter appear.

While I believe this same result could be achieved with just style changes, to keep the styles a little more dumb I've made some structural changes:

  • Nest the drawer more deeply in the interface skeleton (sibling to other sidebars/actions).
  • Move the Navigation toggle into the header (out from of NavigationSidebar)

UPDATE: I've noticed that because of the structural change in this PR the tabbing order improvements from #25884 are undone. I made a POC repair for that with some manual focus management but haven't yet pushed it as I'm first trying to see if the design change is deemed desirable. If so, I'd be interested in trying an alternate CSS only PR to see if that'd be better. For that reason, I'll suggest to @noahtallen to delay an in-depth review unless you just happen to get enthusiastic about this approach.

Static comparison at a medium breakpoint

Before After
edit-site-open-drawer-medium-before edit-site-open-drawer-medium-after

Screen recordings

Top toolbar and wide breakpoint

the lesser end of this breakpoint has issues when a long toolbar (template part in this instance) is present and the drawer is open.

Before
After

Medium breakpoint

Hiding some and moving other items while the drawer opens is distracting.

Before
After

Tangential changes

  • Footer is no longer fixed position, allowing removal of some CSS elsewhere that was accounting for that

How has this been tested?

At all breakpoints opening and closing the drawer. With or without top toolbar mode.

Types of changes

Enhancement?

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@stokesman
Copy link
Contributor Author

For now, it probably doesn't matter how this was implemented and I'm just looking if there's any reason this shouldn't be done from a design perspective. With that in mind, I'd like to ask @jasmussen, @Copons, @jameskoster and @david-szabo97 if they might have a look and comment. Thanks 🙇 and excuse me if this was already tried and discarded some time ago but I didn't find any previous discussions.

@jasmussen
Copy link
Contributor

Thanks so much for all your awesome PRs! I've CC'ed Noah for some context on the technical side, see if there's any CSS optimizations on the way.

Speaking from the design side, I would suggest that the current behavior, even if some of the details could use refinement, is intentional. To me it's about the hierarchy. Reading from left to right, the WordPress menu is at the top of the hierarchy and affects the entire page, therefore it pushes the entire page. Then comes the top toolbar, the canvas, and lastly the block inspector sidebar at the bottom of the hierarchy.

I understand that one goal of this PR is to address the top toolbar configuration being completely broken. That does need to be addressed, but I ultimately believe the way to solve that is to force it to stack below the editor bar. It currently does this on the large breakpoint, but not the wide breakpoint:

Screenshot 2020-12-10 at 09 19 00

The thing is, even this PR doesn't solve the problem with the centered page title dropdown, although it improves it a lot.

@stokesman
Copy link
Contributor Author

Thank you for your thoughts and spirit Joen.

To me it's about the hierarchy ... affects the entire page, therefore it pushes the entire page

I felt like that must be part of it. Here are a couple thoughts around that. A minor thing is that the items in the navigation panel don't always affect the whole page. When changing between items that use the same template, only the content portion is updated. On a different tack, the global styles sidebar might be argued as hierarchically outside the level of the page and yet it pushes only into the canvas area.

the way to solve that is to force it to stack below the editor bar.

Agreed and then some. IMO the top toolbar mode should always stack the block toolbar no matter the viewport width as it makes it more distinct. That it moves to the same row at >= wide breakpoints seems a nigh pointless affordance since at that screen width there's likely plenty of screen height to spare.

@jasmussen
Copy link
Contributor

Agreed and then some. IMO the top toolbar mode should always stack the block toolbar no matter the viewport width as it makes it more distinct. That it moves to the same row at >= wide breakpoints seems a nigh pointless affordance since at that screen width there's likely plenty of screen height to spare.

Let's try that next, then.

I actually tried forcing this to stack in a small CSS only PR, but there's a little more to it than just CSS, and if the top toolbar is to always be stacked, then there's some component stuff that can be refactored, so I shelved it for the time being.

@stokesman stokesman closed this Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants