Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: Remove mention about Safari's limited support flexbox gap #40844

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

coliff
Copy link
Contributor

@coliff coliff commented Sep 17, 2024

Description

This callout was added a while ago and Safari has supported this for a while. It's safe to remove now.

Motivation & Context

Having this callout may cause unnecessary concern.

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Related issues

@julien-deramond
Copy link
Member

Unfortunately, according to our .browserslistrc, we're still supposed to support Safari 12. Based on the data from https://caniuse.com/flexbox-gap only, flexbox gap is only supported starting from Safari 14, though I haven't manually verified this.

Unless I've overlooked something, we have two options here:

  • We label this PR for v6, which aligns with our future update of .browserslistrc
  • We close this PR and either mention this decision or cherry-pick your commit in #37309 for v6.

@coliff
Copy link
Contributor Author

coliff commented Sep 18, 2024

Unfortunately, according to our .browserslistrc, we're still supposed to support Safari 12. Based on the data from caniuse.com/flexbox-gap only, flexbox gap is only supported starting from Safari 14, though I haven't manually verified this.

Unless I've overlooked something, we have two options here:

  • We label this PR for v6, which aligns with our future update of .browserslistrc
  • We close this PR and either mention this decision or cherry-pick your commit in #37309 for v6.

I think rewording the callout could be a better option then. Perhaps something like 'Support for gap utilities with flexbox isn't available in Safari prior to 14.5, so consider verifying your intended browser support. Grid layout should have no issues. Read more.'

The main issue I have is that it currently says "...was recently added to Safari" which is no longer the case.

@julien-deramond
Copy link
Member

I think rewording the callout could be a better option then.
[...]
The main issue I have is that it currently says "...was recently added to Safari" which is no longer the case.

Great, let's proceed with this option. I'll leave it to you to modify the PR. Once it's updated, I'll review and merge it. Then I'll drop the paragraph entirely in #37309 so we don't forget for v6.

This callout was added a while ago and Safari has supported this for a while. so lets update the wording.
Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the enhancement @coliff 🙏

@julien-deramond julien-deramond merged commit 523493d into twbs:main Sep 18, 2024
14 checks passed
@julien-deramond julien-deramond mentioned this pull request Sep 18, 2024
13 tasks
@coliff coliff deleted the patch-1 branch September 18, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants