Skip to content

Conversation

@AlexAndBear
Copy link
Contributor

Description

Related Issue

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 review from Copilot and kulmann May 28, 2025 19:13
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

This PR adds avatar support to the shares view by introducing a new composable for loading avatars, extending configuration to control avatar concurrency, and updating components to render user avatars.

  • Extended the config schema and default options with an avatars setting.
  • Added a useLoadAvatars composable with a request queue for fetching user photos.
  • Updated ResourceTable.vue, UserAvatar.vue, and OcAvatars.vue to render avatars via slots.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/web-pkg/src/composables/piniaStores/config/types.ts Added avatars to the Zod config schema
packages/web-pkg/src/composables/piniaStores/config/config.ts Set default avatars concurrency in options
packages/web-pkg/src/composables/index.ts Exported the new avatars composable
packages/web-pkg/src/composables/avatars/useLoadAvatars.ts New composable to enqueue and fetch avatars
packages/web-pkg/src/composables/avatars/index.ts Re-export of useLoadAvatars
packages/web-pkg/src/components/FilesList/ResourceTable.vue Integrated <oc-avatars> slots and UserAvatar
packages/web-pkg/src/components/Avatars/UserAvatar.vue Refactored to use useLoadAvatars instead of raw fetch
packages/design-system/src/components/OcAvatars/OcAvatars.vue Added a named slot for custom avatar rendering
Comments suppressed due to low confidence (3)

packages/web-pkg/src/composables/avatars/useLoadAvatars.ts:1

  • [nitpick] New composable useLoadAvatars is not covered by tests. Consider adding unit tests to verify queue behavior and error handling.
import PQueue from 'p-queue'

packages/web-pkg/src/components/FilesList/ResourceTable.vue:736

  • The computed property shareType references an undefined variable shareType. You likely intended to return the imported ShareTypes or remove this redundant computed prop.
shareType() {

packages/web-pkg/src/composables/avatars/useLoadAvatars.ts:10

  • If configStore.options.concurrentRequests.avatars is undefined, PQueue will default to unbounded concurrency. Consider providing a fallback value (e.g. || 1) to avoid accidental overload.
const queue = new PQueue({ concurrency: configStore.options.concurrentRequests.avatars })

Comment on lines 206 to 213
<template #userAvatars>
<user-avatar
v-for="avatar in getSharedByAvatarItems(item)"
:key="avatar.username"
:user-id="avatar.id"
:user-name="avatar.displayName"
:width="30"
/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kulmann
I can't think of a smarter solution here, that won't be blocking (await)

@AlexAndBear AlexAndBear force-pushed the issues/598-followup branch from ccbbdd5 to e469cb9 Compare May 28, 2025 21:09
@AlexAndBear
Copy link
Contributor Author

AlexAndBear commented May 28, 2025

image

I added the avatars to the activity panel, which looks super faaaaaaaaaaaaaaaaaaaaancy 🤩
Anyways, when we look to the entry "shared with users", we will see a little inconsistency because the backend doesn't provide the sharetype

https://github.com/opencloud-eu/opencloud/pull/954/files

@kulmann
Copy link
Contributor

kulmann commented May 30, 2025

image I added the avatars to the activity panel, which looks super faaaaaaaaaaaaaaaaaaaaancy 🤩 Anyways, when we look to the entry "shared with users", we will see a little inconsistency because the backend doesn't provide the sharetype

https://github.com/opencloud-eu/opencloud/pull/954/files

I love it! 😍

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.

Shared with two users and a group, the group is not stacked with the two users, but instead on the side. Also, something is off with the borders. The first item has no border, the stacked items do have a border. I've put another window below and zoomed in a bit, so that it's easier to see.

Screenshot 2025-05-30 at 14 40 54

As soon as I add a third user and the +1 appears, the group is not standalone anymore.

Screenshot 2025-05-30 at 14 41 43

@AlexAndBear AlexAndBear requested a review from kulmann May 30, 2025 14:34
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.

Love it 😍 Especially positively surprised by the inline avatars in the activities panel 🥰

@kulmann kulmann merged commit 5cc04e6 into main Jun 2, 2025
18 checks passed
@kulmann kulmann deleted the issues/598-followup branch June 2, 2025 12:30
@openclouders openclouders mentioned this pull request Jun 2, 2025
1 task
@ScharfViktor
Copy link
Contributor

ScharfViktor commented Jun 2, 2025

a smal question:
in the shares with me or shares with other page user without an avatar appears as a red circle.

Screenshot 2025-06-02 at 15 58 00

I understand that there should be initials? same as here:

Screenshot 2025-06-02 at 15 55 21

@AlexAndBear
Copy link
Contributor Author

this is a bug

@AlexAndBear
Copy link
Contributor Author

https://github.com/opencloud-eu/web/compare/avatar-followup?expand=1

@openclouders openclouders mentioned this pull request Jun 2, 2025
1 task
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.

4 participants