Add sorts #165
Add sorts #165
Conversation
✅ Deploy Preview for carbon-icons-motion ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| <li | ||
| className='icon-tile' | ||
| onMouseEnter={() => setSortAscendingAnimating(true)} | ||
| onMouseLeave={() => setSortAscendingAnimating(false)} | ||
| > | ||
| <h3>Sort ascending</h3> |
There was a problem hiding this comment.
Feel like you should consider creating an IconTile component so if you ever need to change the structure, you won’t need to replace all these <h3>s. Probably feels pretty minor for now, but could come in handy down the road if you only need to make the changes in one place instead of for each instance.
There was a problem hiding this comment.
I agree 110%! It's something I've planned on fixing but haven't gotten to yet. I thought I had an issue open for it but I didn't, so I'll make one now!
| @keyframes sort-remove-vertical { | ||
| 0% { | ||
| transform: scaleY(1); | ||
| } | ||
| 10% { | ||
| transform: scaleY(.7); | ||
| } | ||
| 20% { | ||
| transform: scaleY(1); | ||
| } | ||
| 100% { | ||
| transform: scaleY(1); | ||
| } | ||
| } | ||
|
|
||
| @keyframes sort-remove-horizontal { | ||
| 0% { | ||
| transform: scaleX(1); | ||
| } | ||
| 10% { | ||
| transform: scaleX(.7); | ||
| } | ||
| 20% { | ||
| transform: scaleX(1); | ||
| } | ||
| 100% { | ||
| transform: scaleX(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
Not sure if you’ve run into this before with other icons, but noticed that at 100% (default zoom) this appears rotated (like a + sign) in Chrome. This was not an issue in Safari though so it’s possible that it’s specific to Chrome? As is though, this could be a confusing animation.
(Zoomed in at the end where it does not appear to be an issue anymore.)
There was a problem hiding this comment.
@elycheea Thanks for catching this and for all the help! I can't seem to recreate the glitch in Chrome at 100 zoom or zoomed out/in. Hmm. Weird! I'll keep trying.
@kristastarr Do this glitch happen for you too in chrome?
There was a problem hiding this comment.
Fwiw, I’m in Chrome 108. Probably won’t get a chance to look into it more today, but I definitely did not see this in any of the other close/X icons when I did a quick check.
There was a problem hiding this comment.
Very weird. I don't see the problem zoomed in, out, or at default zoom in Chrome 108, Firefox, or Safari 🤔 I've been doing some reading on transform: rotate() vs rotate. I'm going to try changing to transform: rotate to see if we get a different result - @elycheea after I push the change, would you mind checking Chrome again and see if you are still seeing the issue?
There was a problem hiding this comment.
Well I'm super confused now bc when I make the change from rotate: 45deg to transform: rotate(45deg), I am seeing the issue. I went ahead and pushed the change. @johnbister @elycheea do you both want to check and see how it looks for you now?
There was a problem hiding this comment.
@kristastarr @elycheea I also see the issue now that it has been changed to transform: rotate(45deg)
I still don't can't recreate the issue at any zoom or default in Chrome when it is rotate: 45deg. So weird. Should we change it back?
| .SortRemoveVertical, | ||
| .SortRemoveHorizontal { | ||
| will-change: transform; | ||
| rotate: 45deg; |
There was a problem hiding this comment.
I think usually we use transform: rotate(), but looks like rotate has decent although fairly recent support.
https://developer.mozilla.org/en-US/docs/Web/CSS/rotate#browser_compatibility
There was a problem hiding this comment.
Good article on this topic: https://danielcwilson.com/blog/2020/02/motion-path-transforms/
this closes #132