-
Notifications
You must be signed in to change notification settings - Fork 95
fix(NcAvatar): make min status size visually accessible #7476
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
Conversation
|
So the status icon is hidden if the avatar is below 50px? Then this is a no go as this will break many places, it must at least work with default clickable area (34px) avatar size. |
No, it is just bigger from 11px -> around 14px |
|
Or maybe I read it wrong, it sets the min size? Then we need to fix this in a different way (the original issue) as this then breaks again some places where it was fixed by the original PR. |
|
Also it's cropped on 'After' screenshot 🙈 IMO we should define a minimum size of an icon, and 'break' orbital rule the way, that icon is moving slightly inside the avatar, not going out of square frame |
src/components/NcAvatar/NcAvatar.vue
Outdated
| // Limit the status icon size to the status of a 50px avatar | ||
| // Ideally avatars with a smaller should not be used with the status icon at all | ||
| --avatar-status-size-min: calc(var(--default-clickable-area) * (1 - 1 / sqrt(2))); | ||
| --avatar-status-size-min: calc(50px * (1 - 1 / sqrt(2))); |
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.
This is more than 14px, why a random number like 50px?
I think it makes more sense to use either clickable area large which is a value you might choose for the avatar when not using magic numbers:
| --avatar-status-size-min: calc(50px * (1 - 1 / sqrt(2))); | |
| --avatar-status-size-min: calc(var(--clickable-area-large) * (1 - 1 / sqrt(2))); |
Or because you want to limit the emoji to a readable size, then you do not need the avatar size here at all but just use the min. accessible size:
| --avatar-status-size-min: calc(50px * (1 - 1 / sqrt(2))); | |
| --avatar-status-size-min: var(--font-size-small); |
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.
font size seems to be optimal
Yes that exactly is my problem with this change here, because it will break server styles again. I think the best solution is to limit the icon size to |
Signed-off-by: Dorra Jaouad <dorra.jaoued7@gmail.com>
c3b58f7 to
07e9135
Compare
|
Also, initials overlap the status |
susnux
left a comment
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.
Seems to work fine now also for default clickable area
But not related to this PR, the order of elements is wrong, the status icon element should be the last one. |
Kind of related. It became more visible with the new position and size on the status icon. |
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.
Added lib screenshots. Fixing initials would be moving 10 lines around, so shouldn't be a problem
|
/backport to stable8 |

☑️ Resolves
🖼️ Screenshots
🏁 Checklist
stable8for maintained Vue 2 version or not applicable