-
-
Notifications
You must be signed in to change notification settings - Fork 835
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
CSS Code Housekeeping #3026
CSS Code Housekeeping #3026
Conversation
Flarum seem to have had some kind of user display in the sign up modal on successful sign up, which no longer exists. https://github.com/flarum/core/blob/v0.1.0-beta/js/forum/src/components/SignUpModal.js#L111
I'm definitely in favour of this direction. We should also remove all our vendor mixins ( |
Yes as part of this PR and not others if you see what I mean :P |
Actually, removing the mixins would break any extensions using them 🤔 I'll just remove their usage for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to remove the usages of translate3d. We can use normal translate instead.
This was a hack to force a separate compositing layer which was GPU optimised, but modern browsers do this automatically if transform is animated in any way in CSS.
We should also remove the unneeded ~""
surrounding some of the property values.
Couldn't be modify the mixing to only create the unprefixed version of each of the properties? That way extensions using them don't break, but they don't have the vendor prefixes either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about the multiple reviews. My fat fingers kept pressing the wrong button on mobile.
Co-authored-by: David Wheatley <hi@davwheat.dev>
I couldn't find anything online about this to make sure it's no longer necessary :/
The deprecated mixins here are all vendor-less though, you removed them in a previous PR. |
# Conflicts: # less/common/Badge.less
Take a look at this: https://www.smashingmagazine.com/2016/12/gpu-animation-doing-it-right/#use-css-transitions-and-animations-whenever-possible Elements with transform properties, transitions with transform, animations with keyframes that contain transform, or
Oops. I forgot I did that. 😅 |
This should be enough for now. |
This reverts commit af89b23.
Progresses #2600
Changes proposed in this pull request:
General cleanup of certain bits of code, all safe and doesn't change anything.
Reviewers should focus on:
🤷🏼♂️
Confirmed