Skip to content

Conversation

@AlexAndBear
Copy link
Contributor

@AlexAndBear AlexAndBear commented Jun 3, 2025

Description

Before
image

After
image

Related Issue

  • Fixes <issue_link>

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Types of changes

  • Bugfix
  • Enhancement (a change that doesn't break existing code or deployments)
  • Breaking change (a modification that affects current functionality)
  • Technical debt (addressing code that needs refactoring or improvements)
  • Tests (adding or improving tests)
  • Documentation (updates or additions to documentation)
  • Maintenance (like dependency updates or tooling adjustments)

@AlexAndBear AlexAndBear requested a review from kulmann June 3, 2025 20:44
@AlexAndBear AlexAndBear self-assigned this Jun 3, 2025
Copilot AI review requested due to automatic review settings June 3, 2025 20:44
Copy link
Contributor

Copilot AI left a 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 avatarsRef template reference to the avatars container
  • Adds an onMounted hook to set decreasing z-index on 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 using ref<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 observer variable 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 onMounted hook always applies z-index styling; it should be guarded (e.g., only run when stacked is true) to avoid unnecessary work when avatars aren’t stacked.
onMounted(() => {

const avatarElements = Array.from(unref(avatarsRef).children)
avatarElements.forEach((child, index) => {
Copy link
Contributor

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.

Copy link
Contributor Author

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 🤔

@AlexAndBear AlexAndBear requested a review from kulmann June 4, 2025 08:46
@AlexAndBear
Copy link
Contributor Author

@kulmann I think it's finally time to close the chapter avatars ;)

@AlexAndBear AlexAndBear force-pushed the oc-avatars-change-direction branch from af9e527 to fb94a04 Compare June 5, 2025 05:50
Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! 🦄

@AlexAndBear AlexAndBear merged commit d0dc970 into main Jun 5, 2025
18 checks passed
@AlexAndBear AlexAndBear deleted the oc-avatars-change-direction branch June 5, 2025 07:32
@openclouders openclouders mentioned this pull request Jun 5, 2025
1 task
openclouders pushed a commit that referenced this pull request Jun 5, 2025
Co-authored-by: Benedikt Kulmann <b.kulmann@opencloud.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants