-
Notifications
You must be signed in to change notification settings - Fork 25
feat: change visual representation of stacked avatars #793
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
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.
Pull Request Overview
Refactors the stacked avatars component to dynamically assign z-index values for proper layering.
- Introduces a
avatarsReftemplate reference to the avatars container - Adds an
onMountedhook to set decreasingz-indexon each avatar element - Imports new Vue APIs (
useTemplateRef,unref,onMounted)
Comments suppressed due to low confidence (3)
packages/design-system/src/components/OcAvatars/OcAvatars.vue:39
- Vue does not export
useTemplateRef; consider usingref<HTMLElement|null>()for template refs instead of a nonexistent API.
import { computed, onMounted, unref, useTemplateRef } from 'vue'
packages/design-system/src/components/OcAvatars/OcAvatars.vue:110
- The
observervariable is declared but never used; it can be safely removed to reduce clutter.
const observer = null
packages/design-system/src/components/OcAvatars/OcAvatars.vue:169
- The
onMountedhook always applies z-index styling; it should be guarded (e.g., only run whenstackedis true) to avoid unnecessary work when avatars aren’t stacked.
onMounted(() => {
| const avatarElements = Array.from(unref(avatarsRef).children) | ||
| avatarElements.forEach((child, index) => { |
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 can avoid the repaint. You can give the desired z-index as property to all the avatars individually and then inside oc-avatar use the prop for a css variable.
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 am not sure if this is possible, since we have a slot for #userAvatars 🤔
|
@kulmann I think it's finally time to close the chapter avatars ;) |
af9e527 to
fb94a04
Compare
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.
Awesome! 🦄
Co-authored-by: Benedikt Kulmann <b.kulmann@opencloud.eu>
Description
Before

After

Related Issue
How Has This Been Tested?
Types of changes