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

Safari styling errors #435

Closed
Irev-Dev opened this issue Aug 3, 2021 · 18 comments · Fixed by #451
Closed

Safari styling errors #435

Irev-Dev opened this issue Aug 3, 2021 · 18 comments · Fixed by #451
Assignees
Labels
help wanted Extra attention is needed tailwindcss

Comments

@Irev-Dev
Copy link
Owner

Irev-Dev commented Aug 3, 2021

I've largely neglected safari and the result is there are a bunch of styling problems
image

Z issues for the login modal

image
lots of flex gaps ignored by the browser and the page leaves a gap at the bottom of the page.

image
More ignored gaps.

found by @revolter.

@Irev-Dev
Copy link
Owner Author

Irev-Dev commented Aug 3, 2021

yup so safari just ignores flex gaps until this year, but we probably need to refactor using padding to support older safari.
https://css-tricks.com/safari-14-1-adds-support-for-flexbox-gaps/

@Irev-Dev Irev-Dev added good first issue Good for newcomers help wanted Extra attention is needed tailwindcss labels Aug 7, 2021
@Irev-Dev
Copy link
Owner Author

Irev-Dev commented Aug 7, 2021

Because this is all style related it would be a good first issue for someone, especially if you've used tailwind before.

@Irev-Dev Irev-Dev removed the good first issue Good for newcomers label Aug 7, 2021
@Irev-Dev
Copy link
Owner Author

Irev-Dev commented Aug 9, 2021

Wait this is a terrible issue for you right @franknoirot? don't you run Windows?

@franknoirot
Copy link
Collaborator

@Irev-Dev I've got a MacBook for work so I think I can do it alright!

@Irev-Dev
Copy link
Owner Author

Irev-Dev commented Aug 9, 2021

Perfect.

@franknoirot
Copy link
Collaborator

@Irev-Dev confirmed I've been able to get my dev environment in order and use Redwood's forwarding dev command to open the site running locally from my work computer in Safari. On the hunt for some solutions, think I understand the z-index issue and will investigate the rest tomorrow.

@franknoirot
Copy link
Collaborator

Okay so the problem with the gaps is that the gap CSS property is only supported in the absolute latest Safari version, which up until now only supported gap or the former grid-gap for elements with display: grid and no gap-like property for elements with `display: flex'.

Typically you could use an @supports (gap: 1rem) statement to detect if a browser supports a CSS property, but you can't in this case because it's a two condition support statement targeting Safari: supporting gap: 1rem in tandem with display: flex. This has sparked some fascinating discussion on the W3C CCSWG even though this particular iteration of the problem will go away as Safari users upgrade.

In the meantime, there is no CSS-only fix for this solution. As noted in the CSSWG thread, Ahmad Shadeed published a JS solution for detecting support in this blog post that explains the detection issue.

@Irev-Dev
Copy link
Owner Author

Nice investigative work @franknoirot 🕵️ ,
I was thinking that we would solve this simply by replacing the gap class currently on the parent with padding/margin on the children, bit of a pain but also very straight forward?
Or do you think that a more sophisticated solution detecting the problem and adjusting is called for?

Actually would using grid instead of flex work in most of our cases? I don't think we're using flex specific features (like grow/shrink) so maybe that would work?

@Irev-Dev
Copy link
Owner Author

https://twitter.com/fra_michelini/status/1425023649069867009

@franknoirot
Copy link
Collaborator

@Irev-Dev yeah I'd like to see where I can get away with just converting from flex to grid and the like.

One potential reason for using the more bloated detection patch is we can remove that once support is widespread enough and have the best practice already sitting there. And since flex containers don't have collapsing margins any margin we add to their children could cause misalignment due to margins on the first and last children being applied that we would have to override, and I'm not sure if there are Tailwind classes for child selectors so those overrides will likely be custom styles anyway. But I'll do some research tonight.

@franknoirot
Copy link
Collaborator

Ugh @Irev-Dev I ran into a problem tonight. At work we keep our laptops up-to-date with the latest iOS (I'm running 14.5) and you can't downgrade, so I'm not able to recreate the gap issue. I'm trying to emulate older versions of Safari using XCode (I downloaded iOS 13.5 Simulator), but you can't use that to simulate desktop Safari, only iPad and other mobile devices.

I have been able to confirm that replacing gap TailwindCSS class names with one-sided margins on all but the first/last children is a good replacement that looks the same in other browsers, but I can't recreate the environment to see all of the issues for real right now.

Do you know of a reliable way to emulate old desktop Safari versions?

@franknoirot
Copy link
Collaborator

franknoirot commented Aug 11, 2021

One good thing I will say is everything looks great on latest Safari haha, even that height issue with the IDE page is gone for me.

CadHub IDE in Safari 14.1.1

@revolter
Copy link
Contributor

@franknoirot, but can you reproduce this:

image

?

It happens for me in Safari 14.1.2 (16611.3.10.1.3).

@franknoirot
Copy link
Collaborator

@revolter yup I can replace that! I've committed a fix to it in 68346e0

@revolter
Copy link
Contributor

Oooh, I see now. I don't experience the gap issues either.

@Irev-Dev
Copy link
Owner Author

Unless you really want to see this issue to the end @franknoirot, I might as well fix the gap issue since I still got the old version on my machine.

Good that the rest of the issues in the IDE are okay in the new version.

@Irev-Dev
Copy link
Owner Author

Only done one so far but its looking promising
image
image

@franknoirot
Copy link
Collaborator

Nice! Appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed tailwindcss
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants