Site editor: conditionally render global styles revisions footer in sidebar #53204
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
…t so that any side effects from the footer wrapper are not rendered, e.g., styles and what not Ensure that the precise bottom margin persists if the footer isn't rendered
ramonjd
added
[Type] Enhancement
ramonjd
commented
Aug 1, 2023
| className={ classnames( | ||
| 'edit-site-sidebar-navigation-screen__main', | ||
| { | ||
| 'has-footer': !! footer, |
There was a problem hiding this comment.
Seems a bit brute force, but it's all I could think of to ensure the gap is consistent.
The footer needs a top margin to allow the top grey border some room to display clearly.
There was a problem hiding this comment.
While it is a little brute force, I like the clarity of has-footer as a classname, and it's clear what's going on here. Looks good to me for now!
|
Size Change: +35 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
andrewserong
approved these changes
Aug 1, 2023
There was a problem hiding this comment.
Thanks for fixing this up quickly, this is testing great for me!
✅ Spacing looks correct when no footer is rendered
✅ Margin looks correct when footer is rendered
✅ Tested with a theme with no revisions and a theme with some revisions
✅ Double checked each of the other browse mode screens look correct
| No revisions | Some revisions |
|---|---|
 |
 |
LGTM! ✨
| className={ classnames( | ||
| 'edit-site-sidebar-navigation-screen__main', | ||
| { | ||
| 'has-footer': !! footer, |
There was a problem hiding this comment.
While it is a little brute force, I like the clarity of has-footer as a classname, and it's clear what's going on here. Looks good to me for now!
andrewserong
added
the
Backport to WP 6.7 Beta/RC
ramonjd
deleted the
update/site-editor-sidebar-conditionally-show-styles-revision-footer
branch
|
Flaky tests detected in a747337. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5721007950
|
tellthemachines
pushed a commit
that referenced
this pull request
Aug 1, 2023
…t so that any side effects from the footer wrapper are not rendered, e.g., styles and what not (#53204) Ensure that the precise bottom margin persists if the footer isn't rendered
tellthemachines
removed
the
Backport to WP 6.7 Beta/RC
tellthemachines
added a commit
that referenced
this pull request
Aug 1, 2023
* Top toolbar: Fix issues with save button overlap, and plugin buttons (#53101) * Shorten the width of the top toolbar overlay and make doc title smaller. * Add a scrim and a margin to handle plugin buttons better. * Remove block tools back compat component schedule for deprecated in 6.3 (#53115) * Removes usage of BlockToolsBackCompat * Remove unwanted BlockTools from Nav sidebar * Footnotes/RichText: fix getRichTextValues for deeply nested blocks (#53034) * Defer to preceding handlers in command palette keyboard shortcut (#53001) * Image block: fix image size at wide and full width (#53184) * Fix regression with Edit site Navigate regions (#52940) * Make the navigabel region wrap the inert sidebar. * Adjust animation. * Fix not expanding pattern in page editor (#53169) --------- Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> * Footnotes: fix published preview (#53072) * Footnotes: fix published preview * remove var dump * Fix php lint * PHP lint * Address feedback * Add e2e test * Footnotes: disable for synced patterns and prevent duplication for pages in site editor (#53003) * Initial commit: - Prevent footnote creation withing core/block - Only insert a footnote if one isn't found in the entity block list * Try grabbing controlled blocks from parent post content block * Cache `selectedClientId` Get hasParentCoreBlocks using separate useSelect call. * Rename hasParentCoreBlocks to isBlockWithinPattern Add comments * Removing while loop since we're already fetching the post content parent in the `getBlockParentsByBlockName` call above * Reinstating while loop because it can deal with nested blocks --------- Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com> * Footnotes: add missing _ in revision field filter (#53135) * Footnotes: add missing _ in revision field filter * Use correct hook name * Revert prefixing callback names * don't display BlockContextualToolbar at all in contentonly locking (#53110) * Render the footer conditionally in the global styles sidebar component so that any side effects from the footer wrapper are not rendered, e.g., styles and what not (#53204) Ensure that the precise bottom margin persists if the footer isn't rendered * Pattern: Add getBlockRootClientId call (#53206) --------- Co-authored-by: Joen A <1204802+jasmussen@users.noreply.github.com> Co-authored-by: Dave Smith <getdavemail@gmail.com> Co-authored-by: Ella <4710635+ellatrix@users.noreply.github.com> Co-authored-by: Mitchell Austin <mr.fye@oneandthesame.net> Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Co-authored-by: Andrea Fercia <a.fercia@gmail.com> Co-authored-by: Kai Hao <kevin830726@gmail.com> Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Co-authored-by: Ramon <ramonjd@users.noreply.github.com> Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Co-authored-by: Andrei Draganescu <me@andreidraganescu.info>
11 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?
Render the footer conditionally in the global styles sidebar component so that any side effects from the footer wrapper are not rendered, e.g., styles and what not
Ensure that the precise bottom margin persists if the footer isn't rendered
Why?
A top border style is applied to the footer wrapper in
<SidebarNavigationScreen />thanks to.edit-site-sidebar-navigation-screen__footer.If the footer prop is falsy, the footer will not render at all.
How?
Moving select logic from the global styles footer component into the main component
<SidebarNavigationScreenGlobalStyles />so we can conditionally render it.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
With footer
No footer