style(avatar): oversizing of avatar on small screens #757
Conversation
There was a problem hiding this comment.
Thanks @hritvi, and sorry for the delay reviewing!
I tested this today and it's an improvement for tall images, I think it can be improved for wide images.
For example, on a mobile screen, when I first load an image there is still a black border to left even though the image isn't centered and the right is cropped.
If I go into "tablet" mode, then there is a large black space to the right rather than the left:
I suspect CSS isn't the only thing at play here, the cropper sets the top and left of the image and may need updated for mobile.
Also, we really try hard to not use !important within CSS unless absolutely necessary. See https://css-tricks.com/when-using-important-is-the-right-choice/ for a good reference.
|
|
||
| img { | ||
| position: absolute; | ||
| @include respond-to('small') { | ||
| left: (420 - $avatar-size) / 6 !important; |
There was a problem hiding this comment.
Is the !important really needed here? Is there a way to do without?
|
I think @shane-tomlinson already had this in review - pending some requested changes |
|
Sorry, I actually got a little busy with my exams stuff. Now that they are over, I would complete it as soon as possible. |
|
I tried to modify the scripts written in the cropper a lot but nothing changed the position of the image. I guess that works when the image is zoomed or something like that. I even tried to change the horizontal and vertical gutter in avatar_crop but that wasn't working too. I guess we have to modify some code written in the CSS file. I figured out that it seems to work perfectly for screen sizes greater than 420. The only reason I could figure out for it was that the code was written in a way to support screen sizes greater than 420px and that's why 420 is taken as the base to distribute the left and right padding. |
|
@shane-tomlinson @lmorchard can I get some insights into this? |
|
We have decided to put this on hold for now to focus on the React project. We can re-open this PR if/when we get back to it. |
fixes #661
ref #7051.
For a tall sample image:
@lmorchard could you please review it now. Thanks!!