Fix Safari 13 metaboxes from overlapping the content. #33817
Merged
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
jasmussen
added
[Type] Bug
This was referenced Aug 2, 2021
|
Size Change: -29 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
jasmussen
changed the title
|
For folks testing, enabling custom fields is under the three dot post-editor options:
|
gwwar
approved these changes
Aug 4, 2021
There was a problem hiding this comment.
Thanks @jasmussen! I tested this on browser stack w/ Safari 13, and also did spot checks with Safari 14, FF, Chrome.
|
Thank you. In my testing this was solid as well, so I'm going to land it, but give a heads-up in #core-editor to keep an eye out on any potential side effects. |
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
Fixes #33668.
Flex is a delicate feature, especially when supporting a range of browsers. As it turns out, older versions of Safari have a pretty specific interpretation of how flex children should inherit the height of their parents. Which is to say, they do if you don't provide a percentage height.
It gets a bit more gnarly than that, but this SO comment elucidates a bit:
I have developed this patch while using Browserstack to test Safari 13, because I'm no longer on MacOS Catalina. And as best I can tell the flex rules we have in place caused a sequence of events there:
.edit-post-visual-editorwas allowed to shrink, which happened when a metabox was present. This was fixed by disallowing shrinking (flex: 1 0 auto;instead offlex: 1 1 auto;) and by removingflex-basis: 100%;(more on that in a second).edit-post-visual-editor__content-areano longer expanded to fill the void. Presumably because whenflex-basis: 100%;was removed, inheritance there no longer worked. This was fixed by removing the explicitheight: 100%;inline style applied to that element, and by allowing it to grow (flex-grow: 1;).To summarize it to the best of my understanding: when we used percentages, the editing canvas didn't actually expand, causing the metaboxes to overlap it. By removing the percentages, I had to remove them from quite a few of the child items, and change those to rely in the intrinsic
align-items: stretch;property they are given bydisplay: flex;by default.How has this been tested?
In my testing, this threads the needle and accomplishes what it needs to in modern browsers, Chrome, Firefox, Edge and Safari 14. It also makes it all work with Safar 13. I have not tested older yet, but presume it works the same.
The behavior to test is:
Screenshots
The Safari 13 issue that ships in trunk (big GIF):
https://cldup.com/_zYngFvNTq.gif
Safari 13 after this PR is applied:
Chrome after this PR applied (should be like before):
Types of changes
Since this is a pretty sensitive part of the editor layout code, it's important to test this pretty broadly, notably with those of you with access to Safari 13. But it's still important to ensure identical behavior on modern browsers as well.
Checklist:
*.native.jsfiles for terms that need renaming or removal).