Try: Use column gap for button blocks #31386
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
kjellr
added
[Type] Enhancement
|
Size Change: +304 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
|
This is great, this is beautiful, themers are going to love it:
At some point once the dust settles, we should see if this gap could become a global styles property; since it's so easy to change and resilient, it would be nice to handle in such a neat way. I tested the justifications. Left looks good:
Space between looks amazing:
Center is good:
Right justified is given some left margin which is possibly/probably unnecessary now?
So happy we can land this now. Thank you Kjell! |
Good catch! I've pushed a small update to zero that out. How do you feel about this aspect of the problem here:
I'm having trouble deciding how much of an issue that is here. 🤔 I'm sure there are some folks stuck on older versions of Safari, and this will probably look like a breaking change for them. But because of that Safari bug, I'm just not sure there will ever really be a way around that. |
|
It's been a while since I investigated safari update mechanisms; can anyone get stuck on an old version of Safari? And if yes, can we find any browser stats to look at? If it's mostly a matter of people updating their browsers/systems, I'd personally feel the pros to this approach vastly outweigh the downsides, which are mostly visual. |
@iamthomasbishop or @mattmiklic: You may know a little about this on the iOS side at least: In general, the PR here is made possible by a Safari fix that is only available in last week's iOS + MacOS releases. Are large portions of the WordPress app's stuck on old iOS versions? Or do they tend to update relatively quickly? On the Mac side, I'm less clear how we'd be sure. I can do a little digging tomorrow to figure it out. |
|
We dropped support for iOS 12 in January once we had over 98% of users on iOS 13 or higher. Generally we find that iOS users update pretty quickly. The same is not necessarily true on Android where we still support versions back to 5.0 (from 2014). Your mileage may vary on the web. 😃 |
|
I found this handy Safari version history on Wikipedia — though it doesn't appear to include Safari 14.1 yet. Nevertheless it seems to suggest that at least Mojave versions of the operating system and onwards get Safari updates still, which is all to suggest that if you use Safari on a Mac operating system is more than two years old, you might get sub-optimal multi-button layouts, and anything later than that we should be pretty good pretty soon. I'd be happy to land this as a result. |
jasmussen
approved these changes
May 11, 2021
There was a problem hiding this comment.
Before:
After:
The difference is clear, this change feels worth it. I think we should get it in.
I think before you land this you might want to rebase, I'm seeing an issue that is almost certainly unrelated to this, but just shows the age of the branch.
|
Sounds good, thanks! I just merged in the latest from |
8 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.
Fixes #28468.
This is a redo of #29165. As @mtias pointed out, the latest versions of Safari on Mac (v 14.1) and iOS (v14.5) both fully support
column-gapnow — even when used forflexbox. 🙌 I've tested this on my own devices and it works well.The main consideration here is that for users on older versions of Safari, this will appear broken (as shown in the screenshots in #30449). I don't think there's any way around that given the issue described in #30449.