Update fixed block toolbar #52123
Merged
Update fixed block toolbar #52123
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
draganescu
added
[Type] Bug
|
Size Change: +87 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
|
Great first step, thanks for working on this:
The double chevrons work well here. I'd love to animate them in the future. I think the important next piece is uncovering undo/redo! Thanks Andrei. |
|
@jasmussen it's kinda complete. The show button labels preference really needs some love. I think you did a great job polishing this interaction, it makes a lot more sense now! 👏🏻 Let me know how it tests for you. Edit: The collapsing in the post editor is a bit useless as things stand now ... |
draganescu
force-pushed
the
fix/top-toolbar-position-and-expanding
branch
from
d5e168b
to
6c727de
Compare
|
Awesome work, this is feeling very good to me, I think this fixes the pressing issues while keeping the core of it. A few small things; this vertical separator: 
It's both a little tall, and hanging a bit low. Essentially it should be identical in placement and height as the separator that sits between toolbar groups: 
Secondly, I agree we can do better with the tooltips. Here's "Show block tools": Here's "Show document tools". Since the top toolbar is always going to be about document tools, and since we are leaning into the expand/collapse pattern of the chevrons, how about we just go with:
Then it's essentially one button with tooltips describing its state. Since that is already how focus is handled (it stays on the same button), it would seem a better switchup of the tooltips, keeping the topic the same. |
|
We have an option in the editor called 
I also see some weird overlapping stuff: 
|
scruffian
reviewed
Jun 30, 2023
| @@ -108,6 +108,9 @@ export default function HeaderEditMode() { | |||
| }; | |||
| }, [] ); | |||
|
|
|||
| const { get: getPreference } = useSelect( preferencesStore ); | |||
| const hasFixedToolbar = getPreference( editSiteStore.name, 'fixedToolbar' ); | |||
There was a problem hiding this comment.
Should we make this a hook so we can share the code?
There was a problem hiding this comment.
To me a two liner is not worth a hok. I would be if the two liner would be some arcane expression or something but this is really basic.
There was a problem hiding this comment.
The same comment as above - https://github.com/WordPress/gutenberg/pull/52123/files#r1252980401.
scruffian
reviewed
Jun 30, 2023
scruffian
reviewed
Jun 30, 2023
|
I think we should probably hide the zoom out mode buttons in this context. I don't actually expect the environment to graduate as is, and most likely it won't have a button in the toolbar if it does. |
I would not let this block this PR. The text labels mode is lacking in other contexts too, such as the inspector. |
|
@scruffian ready for a new review :) |
|
This is almost there, the only thing I noticed is that when you have text labels and you hide the block tools, you see this: 
|
|
Actually sorry I found another few things on smaller screens: 
This is in Firefox (Chrome looks ok). |
|
Flaky tests detected in 5989fa7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5464169932
|
|
@scruffian @jasmussen I've updated the command center button to be smalller on small viewports. I've also made the toolbar scrollable so we avoind as much as possible visible overlapped elements. I've hidden all document tools when show butotn labels is toggled on in the site editor (they're also hidden in the post editor). The option to show button labels really needs its own PR. There's multiple ways to make it look borked. responsive-fixed-toolbar.mp4 |
draganescu
force-pushed
the
fix/top-toolbar-position-and-expanding
branch
from
a3b27fe
to
a4f804c
Compare
|
I can still see some strange overlapping behaviour at some breakpoints: 
I'm happy to go ahead and merge this if you want to deal with them in a followup? |
|
There is no immediate solution, the way we manage the UX is with fixed breakpoints for mobile, tablet, desktop and large desktop. It's likely inbetween there will be overlapping. The only way to fix this is to have a flex container and a way for the scrollbar to overflow nicely. We don't - at least yet. |
scruffian
approved these changes
Jul 4, 2023
draganescu
force-pushed
the
fix/top-toolbar-position-and-expanding
branch
from
74e6c90
to
c34c8fe
Compare
draganescu
force-pushed
the
fix/top-toolbar-position-and-expanding
branch
from
c34c8fe
to
31ede3c
Compare
Mamaduka
reviewed
Jul 5, 2023
Comment on lines
+40
to
+41
| const { get: getPreference } = useSelect( preferencesStore ); | ||
| const hasFixedToolbar = getPreference( 'core/edit-post', 'fixedToolbar' ); |
There was a problem hiding this comment.
This isn't a good way to use selectors when value is used during the render. When the fixedToolbar preference value changes, this won't rerender the component.
Let's make it part of the useSelect below.
There was a problem hiding this comment.
I made a PR for this: #52332
There was a problem hiding this comment.
There's another one in packages/edit-site/src/components/header-edit-mode/index.js, let fix both in one go :)
|
I found one minor design issue. The toolbar overlaps the "Saved" status button in the post editor. Screenshot
|
|
oh @Mamaduka the joy of this implementation 👯 |
tellthemachines
pushed a commit
that referenced
this pull request
Jul 7, 2023
* update the icons for expanding and collapsing the fixed block toolbar * hide the tools selector item in document tools for fixed toolbar preference * reveal undo, redo and list view buttons * tweaks for show icon labels and hide zoom out for top toolbar option * improve the responsiveness of the fixed block toolbar * remove the overflow rule - bad experiment * update top toolbar test with the new label for buttons * update the toolbar tests to account for moving the collapse button
|
I just cherry-picked this PR to the update/first-pre-beta-4 branch to get it included in the next release: 7398771 |
tellthemachines
removed
the
Backport to WP Beta/RC
tellthemachines
added a commit
that referenced
this pull request
Jul 7, 2023
* add hint to show template part move (#52395) * Page Content Focus: Ignore page content within a Query Loop block (#52351) * Block Editor: Pass context and className props to editor.BlockEdit filter * Page Content Focus: Ignore content blocks that are within a Query Loop * Patterns: stop endless snackbars appearing (#52012) * Patterns: Distinguish between theme patterns and template parts in category list (#52382) * Allow opt out of auto-creation of Navigation fallback (#52319) * Update welcome guide copy (#52282) * Patterns: Update pattern copy (#52340) * Post Title: The changes should be reflected when previewing a post (#52369) * Update fixed block toolbar (#52123) * update the icons for expanding and collapsing the fixed block toolbar * hide the tools selector item in document tools for fixed toolbar preference * reveal undo, redo and list view buttons * tweaks for show icon labels and hide zoom out for top toolbar option * improve the responsiveness of the fixed block toolbar * remove the overflow rule - bad experiment * update top toolbar test with the new label for buttons * update the toolbar tests to account for moving the collapse button * Drop PHP 5.6 CI jobs (#52345) * Remove PHP 5.6 PHPUnit CI job * Raise version in phpcs / WPCS * Patterns: Add handling of sync status to the wp-admin patterns list page (#52346) * Exit template focus when opening the W menu (#52235) * wrap buttons (#52249) * Update the behavior of the cached undo/redo stack (#51644) Co-authored-by: Ella van Durpe <ella@vandurpe.com> * Adjust top position (#52248) * Patterns: add a hint about the rename of reusable blocks to menu and inserter (#51771) Co-authored-by: Saxon Fletcher <saxonafletcher@gmail.com> * Site Editor: update headings hierarchy in the 'Manage all' screens (#52271) * This commit: - updates heading levels on the template and template part pages - passes a `level` prop to Header from Page * update h2 size * Rolling back custom sizes * Rolling back unnecessary classNames There was a rogue space in trunk. Let's let it live * Check randomizer experiment is enabled before rendering button (#52306) * Hide parent selector when parent is 'disabled' or 'contentOnly' (#52264) * Fix incorrect aria-describedby attributes for theme patterns (#52263) * Patterns: rename sync_status and move to top level field on rest return instead of a meta field (#52146) * Fix default block dimensions visibility (#52256) * core/heading * core/details * core/list * core/table * core/video * core/verse * core/social-links * core/site-title * core/site-tagline * core/site-logo * core/post-time-to-read * core/gallery * core/code * core/categories * core/audio * core/archives * Patterns: Display all custom template part areas in sidebar nav (#52355) * Revert phpcs back to PHP 5.6 (#52384) Reverts phpcs PHP compatibility version back to 5.6. * Check if experiment enabled fr this time (#52315) * Navigation: Remove one preloaded endpoint (#52115) * default to showing status (#52226) * Command palette: rename (#52153) * Revise use of “command menu” to “command palette”. Dropping "global" where it was used as well. * Find “command center” and replace with “command palette” * Image block and behaviors: Fix some warnings (#52109) * Fix first warning * Fix second warning - dividing a NaN * Turn off DFM for style book and style editing (#52117) * Add confirmation step when deleting a Template (#52236) * [Command Palette]: Remove suggestion for deleting templates/parts (#52168) * Update stepper styling in Home template details panel (#51972) * Update stepper styling * Remove !important * [Edit Post]: Add toggle fullscreen mode and list view commands (#52184) * Style Book: Show tabs and make blocks clickable when entering edit mode from the Styles menu (#52222) * Style Book: Show tabs and make blocks clickable when entering edit mode from the Styles menu * Move lines * !important (#52025) * Navigation in Site View: Readd the edit button (#52111) * Fix UnitControl crashing on regex control characters. Units are now escaped using `escapeRegExp` before concatenating them into a regular expression. Fixes #52211. --------- Co-authored-by: Mitchell Austin <mr.fye@oneandthesame.net> * Patterns: rename wp_block sync_status postmeta to wp_pattern_sync_status (#52232) --------- Co-authored-by: Kai Hao <kai@kaihao.dev> * Site Editor Frame: Ignore Spotlight in view mode (#52262) * Guide: Place focus on the guide's container instead of its first tabbable (#52300) * Guide: Place focus on the guide's container instead of its first tabbable * Update CHANGELOG * Post editor: Require confirmation before removing Footnotes (#52277) * Post editor: Require confirmation before removing Footnotes Context: #52176 * BlockRemovalWarningModal: Limit width to 40rem * Explain that footnotes are preserved. Add warning to Site Editor * Fix react-dropdown-menu version to avoid breaking change from one of … (#52356) * Fix react-dropdown-menu version to avoid breaking change from one if its dependencies. * Changelog update * move changelog entry to the right place * Update package-lock --------- Co-authored-by: Saxon Fletcher <saxonafletcher@gmail.com> Co-authored-by: Robert Anderson <robert@noisysocks.com> Co-authored-by: Glen Davies <glendaviesnz@users.noreply.github.com> Co-authored-by: James Koster <james@jameskoster.co.uk> Co-authored-by: Dave Smith <getdavemail@gmail.com> Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com> Co-authored-by: Andrei Draganescu <me@andreidraganescu.info> Co-authored-by: Tonya Mork <tonya.mork@automattic.com> Co-authored-by: Riad Benguella <benguella@gmail.com> Co-authored-by: Ella van Durpe <ella@vandurpe.com> Co-authored-by: Ramon <ramonjd@users.noreply.github.com> Co-authored-by: Kai Hao <kevin830726@gmail.com> Co-authored-by: Rich Tabor <hi@richtabor.com> Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Co-authored-by: Ben Dwyer <ben@scruffian.com> Co-authored-by: Mitchell Austin <mr.fye@oneandthesame.net> Co-authored-by: Carlos Bravo <37012961+c4rl0sbr4v0@users.noreply.github.com> Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com> Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Co-authored-by: Timothy Jacobs <timothy@ironbounddesigns.com> Co-authored-by: Kai Hao <kai@kaihao.dev> Co-authored-by: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
|
Post editor - overlaps Update button when plenty of space is available: |
|
@ktmn there is an incoming fix for the post editor. Thanks for the testing! 👏🏻 |
15 tasks
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.
What?
Closes #51711
Why?
To improve the UX of using fixed block toolbar by:
How?
Testing Instructions
Screenshots or screencast
top-toolbar-updates.mp4