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

Remove unneeded vendor prefixes for box-shadow #1763

Merged

Conversation

adamzap
Copy link
Contributor

@adamzap adamzap commented Nov 23, 2024

Browser versions that made these vendor prefixes obsolete were released by the middle of 2011: https://caniuse.com/css-boxshadow.

This is the first in what could be a long series of CSS/JS simplification PRs if this kind of thing is desired by the maintainers. I think we can reduce some complexity by taking small steps like this.

For instance, next I would want to factor the two affected mixins out. They are each used in one place.

@adamzap adamzap force-pushed the remove-unneeded-box-shadow-vendor-prefix branch from f78e392 to f043117 Compare November 23, 2024 01:43
Browser versions that made these vendor prefixes obsolete were released
by the middle of 2011: https://caniuse.com/css-boxshadow.
@adamzap adamzap force-pushed the remove-unneeded-box-shadow-vendor-prefix branch from f043117 to 513287e Compare November 23, 2024 01:44
Copy link
Member

@bmispelon bmispelon left a comment

Choose a reason for hiding this comment

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

This seems like a good change, thanks for submitting it 👍🏻

Thanks also for linking to caniuse, it makes reviewing easier.

In terms of further PRs, I'm looking forward to them and I think we could increase the scope a little bit. I checked with git grep -F -- '-moz-' '*.scss' and that yields about 20 results, which I think should be reviewable in a single PR (assuming all of these vendor prefixes can be removed, which might be incorrect).

I also wonder if there's a tool we could use to automatically flag these kinds of outdated CSS (having a linter we could run in CI would be amazing). I don't mind reviewing PRs like these one by one, but I'd prefer we could do it all in an automated single pass if that's possible.

@bmispelon bmispelon merged commit 3851b2f into django:main Nov 24, 2024
4 checks passed
@adamzap adamzap deleted the remove-unneeded-box-shadow-vendor-prefix branch November 24, 2024 18:47
@adamzap
Copy link
Contributor Author

adamzap commented Nov 24, 2024

@bmispelon Ok thanks! I'll try to increase the scope while keeping different efforts separate, like removing vendor prefixes versus factoring Sass utilities out.

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 this pull request may close these issues.

2 participants