Gap block support: force gap change to cause the block to re-render (fix Safari issue) #34567
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
andrewserong
added
[Type] Enhancement
andrewserong
changed the title
|
Size Change: +1.36 kB (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
youknowriad
reviewed
Sep 6, 2021
| // In Safari, changing the `gap` CSS value on its own will not trigger the layout | ||
| // to be recalculated / re-rendered. To force the updated gap to re-render, here | ||
| // we replace the block's node with itself. | ||
| if ( ref.current ) { |
There was a problem hiding this comment.
Is there a way to limit it to only Safari cause it sounds a bit aggressive :)
There was a problem hiding this comment.
Very good point! I'll add in some user agent sniffing tomorrow 🙂
There was a problem hiding this comment.
@youknowriad I've added in some user agent sniffing here, consistent with this MDN article for detecting Safari (string contains "Safari" but doesn't contain "Chrome" or "Chromium").
It appears to work okay in my testing. I thought of abstracting the check out to somewhere else (perhaps in a package somewhere), but it doesn't look like we're using an explicit UA check for Safari anywhere else, so I thought I'd leave it inline here for now, and until we have a second use for it somewhere. But, do let me know if you can think of a better way to handle the check. Thanks!
youknowriad
reviewed
Sep 6, 2021
|
Thanks for reviewing, Riad! I think when applied to the Group block, because the Layout styling includes updating |
andrewserong
deleted the
fix/block-gap-support-synchronous-update-in-safari
branch
fullofcaffeine
added a commit
that referenced
this pull request
Sep 9, 2021
* trunk: (214 commits) Fix snackbar overflow on nav editor (#34661) Mobile - Allow disabling text and background color via theme.json (#34633) Fix disabled blocks logical error on Widgets screen (#34634) [Mobile] - Global styles - Add support to render font sizes and line height (#34144) ESLint Plugin: Update eslint jsdoc dependency (#34338) Scripts: Add CHANGELOG entry for `jest-dev-server` upgrade (#34657) Bump jest-dev-server to v5 (#34560) Refactor the `core-data` store to thunks (#28389) Only capture toolbars on parent Nav block when not in vertical mode (#34615) Update Changelog for 11.5.0-rc.1 Bump plugin version to 11.5.0-rc.1 Gallery block: Fix media placeholder height in site editor (#34629) Border Controls: Display color indicator and check selected color (#34467) Remove horizontal and vertical navigation block variations from inserter (#34614) AlignmentMatrixControl : Fix/update docs (#34624) Gap block support: force gap change to cause the block to re-render (fix Safari issue) (#34567) [Block Library - Social Links]: Use the new `flex` layout (#34493) [Mobile] Update the bottom sheet header (#34309) Group block: Add a row variation (#34535) Migrate entities.js to thunks (#34582) ...
ntsekouras
reviewed
Dec 21, 2021
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.
Description
During testing of the block gap support in #33991 it was uncovered that updating the CSS value for
gapon its own, doesn't appear to cause a layout to be recalculated / re-rendered by Safari desktop. Adjusting other CSS values at the same time appears to make Safari re-render (e.g. adjusting height or width). This means that in many cases when adjusting gap values in Safari, the block doesn't appear to update, until the user clicks away from that block.To force the browser to re-render the block, in this PR we take a heavy-handed approach and replace the DOM node for the block that has opted-in to the blockGap support with itself when the gap value changes. The has a performance cost, but the hunch that I'm going with is that it's better for Safari to be able to render gap changes live when making changes to the gap value, than not at all.
A note on implementation: I haven't used the
replacemethod in the@wordpress/dompackage, because that method removes and then appends to the parent node, which throws an error since here we're replace the node with itself, soreplaceChildis a better fit. Also, I opted for replacement instead of hacking in a separate CSS update, to avoid having to register asetTimeout. Very happy to hear other ideas for workarounds if folks have them!The issue this PR addresses appears to only be at play in Safari browser.
How has this been tested?
Manually, following the steps in #33991 to opt-in to blockGap support. Broadly that includes:
Switch on the block gap support
theme.jsonfile, undersettings.spacingadd the"blockGap": trueproperty. E.g. this is how my spacing settings look:packages/block-library/src/buttons/block.jsonadd thegap"spacing" support. In my testing, the "supports" key of this file looks like this:packages/block-library/src/buttons/style.scssat line 8 add the following to set the gap styles:Screenshots
In the below screenshots, this is a Buttons block added as the first block in a post, using the Safari browser.
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.jsfiles for terms that need renaming or removal).