Show the admin bar in the Post and Site Editor by default#79197
Show the admin bar in the Post and Site Editor by default#79197fushar wants to merge 15 commits into
Conversation
|
Size Change: -13.4 kB (-0.18%) Total Size: 7.49 MB 📦 View Changed
|
5ba21e9 to
c9aae8c
Compare
|
I'm happy to support this. I think there will be some future followups with the document title field that we can look at separately, but not something that needs to block this. In that sense, this generally has my support. However as this is a larger change, I think we ultimately need some green checks from them. Such as @mtias, @youknowriad. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @lucasmendes-design, @rectang. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
tyxla
left a comment
There was a problem hiding this comment.
Thanks for addressing the feedback @fushar 🙌
Would be nice to get a last thorough test in all screens, across various screen sizes, to ensure everything works correctly and as expected. I have only done partial testing in the meantime.
Otherwise, my biggest concerns have been addressed already. Makes sense to ask other reviewers who had feedback to re-review.
| } | ||
|
|
||
| &.is-fullscreen-mode { | ||
| @include break-medium { |
There was a problem hiding this comment.
Noticing that the corresponding distraction-free rule in the site editor has no breakpoint, while this one is desktop-only. Is this divergence intentional?
There was a problem hiding this comment.
It was, but I think now it's best if they are consistent. The issue is in Post Editor, we always show the admin bar in mobile; the fullscreen mode takes effect only in Desktop 🤔 (hence the breakpoint in the CSS).
With 1b3339e I made Site Editor consistent with the existing Post Editor behavior.
| if ( previousCanvaMode === 'edit' ) { | ||
| toggleRef.current?.focus(); | ||
| const desktopToggle = sidebarRegionRef.current | ||
| ? focus.tabbable.find( sidebarRegionRef.current )[ 0 ] |
There was a problem hiding this comment.
Any chance that the first tabbable element in the sidebar may be different because of a plugin doing something? Should the target be a bit more explicit?
There was a problem hiding this comment.
I'm a bit stuck here. Do you have any suggestion? I'd like to target the left chevron (in < Design) but the back button is not part of the layout, so I can't pass the ref like the SiteHubMobile here. Is it OK if I just target that element via DOM query selector here? 🤔
There was a problem hiding this comment.
Hmm, a DOM query selector would make things worse. Maybe best to keep it as-is, but just add a comment that we're expecting that the first tabbable element is the Back button. Thanks!
| align-items: center; | ||
| justify-content: center; | ||
| background: var(--wpds-color-background-surface-neutral-weak); | ||
| background: transparent; |
There was a problem hiding this comment.
I think we can just remove this line.
| background: transparent; |
There was a problem hiding this comment.
Oops, you're right, fixed in 4516c9f; some missed case when trying to merge the CSS from the experiment.
| align-items: center; | ||
| justify-content: center; | ||
| background-color: hsla(0, 0%, 80%); | ||
| background-color: transparent; |
There was a problem hiding this comment.
I think we can just remove this line.
| background-color: transparent; |
There was a problem hiding this comment.
Same: fixed in 4516c9f; some missed case when trying to merge the CSS from the experiment.
Ah are you still enabling the experiment? Try turning that off for now, I think it's a bug 😅 we're trying to port the logic properly here: WordPress/wordpress-develop#11781 I should remove the duplicated feature in the experiment once the Core PR lands. |
I just tested the following:
The above also applies to Posts in wp-admin and Templates in site admin. So, UX-wise, it is the same action: it goes back to where you're coming from. |
Nice catch, should be fixed via b5bcc83. But the search button (command palette) is not working on trunk either. Clicking it does nothing:
It's because the command palette is not mounted in the site editor v2. I guess it's never been implemented? In any case I think it should be out of scope of this PR. |
Hmm yeah, it can be viewed that way as well. Asking for more input from @lucasmendes-design / @jasmussen. What do you folks think? Should we unify the back buttons in Site Editor and Post Editor to be left chevron ( |
|
@fcoveram hmm but still, the tooltip says |
|
I would go with "Go back" as the tooltip for the action in both editors. "Open navigation" is confusing as open is usually for diving into the details of a content type, and navigation is already a section where nav menus exist. Also, the current "View [section-name]" will conflict the "View Pages" in wp-admin and site admin. |
This is not true, the command palette has always worked on site editor v2 (a recent regression is possible though), what I'm not sure about is whether it was WordPress Core that mounted it automatically like any other admin page or whether it was in the "boot" package. |










Also fixes #57813
What?
This PR shows the admin bar in the Post and Site Editor by default, hiding it ONLY when:
Fullscreen modeandDistraction-free modeare BOTH turned on.Distraction-free modeis turned on (Fullscreen modecan't be disabled here).Now that the admin bar is back which identifies the site context, this PR replaces the now-redundant top-left W/site icon with an explicit back button, resulting in a more obvious navigation control.
Why?
This is part of an effort to make navigation across wp-admin screens more consistent. See: #79083
How?
This essentially "graduates" that part of the feature from the following experiment, moving the logic out of the experiment gating.
Testing Instructions
Test the following scenarios depicted by the following screenshots.
Screenshots or screencast
Post Editor
Site Editor
Site Editor V2
(Enable the
Extensible Site Editorexperiment, then visit Appearance -> Site Editor)Use of AI Tools
I used Claude Code with Opus 4.8 to help migrate the logic out from the experiment gating.