[IMPROVE] The icons in the message box have a transition of color change and zoom on hovering #21106
Conversation
|
@Sing-Li please have a look at this! |
|
@PiyushDhamane You don't need to make changes in the |
|
@aditya-mitra please check |
|
@PiyushDhamane you have deleted package-lock.json and commited the change. While adding files to commit, please don't use Now to correct this mistake, copy paste entire package-lock.json from develop branch and include this in commit, or again do |
|
@yash-rajpal thank you for the elaborate explanation. I've made the commit as directed. Please check |
| &:hover, | ||
| &:focus, | ||
| &.active { | ||
| color: red; |
There was a problem hiding this comment.
Please don't use a static red color, this doesn't go well with the design. Also we have dynamic colors, I mean, user can change colors of UI elements, so we should not provide static color values, instead do it like this. The variable message-box-markdown-hover-color contains the color selected by user.
| &:focus, | ||
| &.active { | ||
| color: red; | ||
| transform: scale(1.5); |
There was a problem hiding this comment.
I don't think its a good idea to use scaling property, It doesn't go well with UI/UX. This is just my opinion. Other places we are doing linear transform, maybe we can do something like that.
There was a problem hiding this comment.
We can use linear transform too, but here we would like to keep the time for transition very small, so that's why I used scaling property.
There was a problem hiding this comment.
Hey @yash-rajpal,
So any other transition except zoom you have in mind? I'll just do it and make the final commit
| @@ -37432,4 +37432,4 @@ | |||
| "dev": true | |||
| } | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
There are still some package-lock.json changes, please remove these changes.
There was a problem hiding this comment.
@yash-rajpal I'm not sure why is this change shown even though I didn't commit it. If you see it says, the brace deleted from 37435 and added again to the same line.
This is a problem I faced in my other commit too.
|
|
Proposed changes (including videos or screenshots)
Fixes ####
Issue(s)
#21070 fixed
Steps to test or reproduce
Further comments