-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[Image banner] Add ambient movement to background #2342
Changes from all commits
3dd6246
58984b1
002c12d
c1a0ec4
51e6579
4b2e85e
4738f82
01dc560
291a05e
20ad896
812104f
b402853
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3120,3 +3120,17 @@ details-disclosure > details { | |
.rte blockquote > * { | ||
margin: -0.5rem 0 -0.5rem 0; | ||
} | ||
|
||
/* Ambient animation */ | ||
|
||
@media (prefers-reduced-motion: no-preference) { | ||
.animate--ambient > img, | ||
.animate--ambient > svg { | ||
animation: animateAmbient 30s linear infinite; | ||
} | ||
|
||
@keyframes animateAmbient { | ||
0% { transform: rotate(0deg) translateX(1em) rotate(0deg) scale(1.2); } | ||
100% { transform: rotate(360deg) translateX(1em) rotate(-360deg) scale(1.2); } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤯 This is neat, I guess it pivots the image around clockwise with a 1em diameter? What about doing If you use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's right! It's pretty fun. The percentage-based Good point about the image size! That could be worth exploring for sure. I'll look into it later this week, but if someone else would like to beat me to it, that's totally cool. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
} |
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.
It was at 40s before, had a feeling it was a little more perceivable without too much in your face. I am curious how others feel about it?
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.
I keep going back and forth. 😄 I'll leave it at this setting for a minute and let folks weigh on whether they think it's too fast or not.
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.
Would using the
.animate--ambient
selector on its own be enough here? Why do we specifically need to targetimg
andsvg
elements?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 need to target the image or svg so that element can rotate around inside of its
banner__media
container. (We can't have thebanner__media
move around inside of the parentbanner
, because when two images are present the 50/50 split would not be preserved throughout the animation).I could have assigned the
.animate--ambient
class directly to the image or svg, but I chose this path so that we can ideally use a singleanimate--{{ section.settings.image_behavior }}
class assignment for all of the differentimage_behavior
selections. While the ambient animation only targets the children of that, the fixed animation in #2345 does target the parent.If there's hesitation around targeting
img
andsvg
specifically, I could assign and target a second class and target that. We don't currently assign a class to the img element in there, so doing this felt more verbose to me.