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

May be time to remove @tailwindcss/aspect-ratio #344

Closed
searls opened this issue Apr 12, 2024 · 2 comments · Fixed by #352
Closed

May be time to remove @tailwindcss/aspect-ratio #344

searls opened this issue Apr 12, 2024 · 2 comments · Fixed by #352

Comments

@searls
Copy link
Contributor

searls commented Apr 12, 2024

This gem includes all of Tailwind's 1st-party plugins, which I think is/was a smart choice, but I just tripped over the fact that @tailwindcss/aspect-ratio doesn't behave like the docs in that aspect-video was doing nothing.

After futzing around for a bit, I realized the gem installs and configures the aspect-ratio plugin, and that was the cause of my issue. Since the plugin was apparently created as a polyfill until Safari 15 released (which did release back in Fall 2021), I think it's probably safe for it to be EOL'd for anyone who isn't targeting ancient browsers

Thoughts?

@flavorjones
Copy link
Member

@searls Thanks for bringing this up. I agree that it's probably time to assume the plugin is probably not useful for more folks.

After a brief read through the plugin's docs, I'm not confident that I know how to make this change in a backwards-compatible way that won't break any existing apps (in particular there seems to be a set of aspectRatio values set by the plugin that aren't set in the core utility); but I don't use this feature set.

If you're comfortable with it, I'd love to see a PR that folks could use and give feedback on.

@searls
Copy link
Contributor Author

searls commented Apr 12, 2024

Well, the biggest concern for me is that the installer will generate a tailwind.config.js that has this plugin listed in it, so any PR that removed the plugin would also break all those generated config files since the require would fail

As a result, I think the least disruptive thing to do would be to remove this line so new installs don't get it by default. Sound good?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants