feat: show recent people on contact icon - #62401
Conversation
7e0645c to
db0b5e6
Compare
| } | ||
| } | ||
| teams.value = [...userTeams] | ||
| await loadPreviewAvatars() |
There was a problem hiding this comment.
Fires twice: setting selectedTeam above already triggers the new watcher. Dropping this call should be enough I think?
| const view = render(ContactsMenu) | ||
| await view.findByRole('button') | ||
|
|
||
| expect(view.container.querySelector('.contactsmenu__trigger-avatars')).toBeNull() |
There was a problem hiding this comment.
Wondering if this runs before loadPreviewAvatars resolves, so it could pass even with the threshold set to >= 1. Perhaps we can add a similar vi.waitFor from the test below?
| margin-inline-end: calc(2 * var(--default-grid-baseline)); | ||
|
|
||
| :deep(.header-menu__trigger) { | ||
| opacity: 1 !important; |
There was a problem hiding this comment.
What is this important working around? To remove the hover/focus feedback?
susnux
added
enhancement
3. to review
db0b5e6 to
def4c5c
Compare
|
@pringelmann Thank you for your comments. |
Signed-off-by: Kristian Zendato <kristian.zendato@nextcloud.com>
def4c5c to
7a3859a
Compare
| return []; | ||
| } | ||
|
|
||
| $entries = $this->manager->getEntries($user, ''); |
There was a problem hiding this comment.
Do I understand correctly that we are now calling this on every single page load? I wonder if retrieving all contacts without a filter like this will impact performance. Would you mind briefly investigating?
There was a problem hiding this comment.
I had a quick look myself: searching with an empty filter takes the recent-statuses path, which does one address book search per recent status, up to 25, then runs every action provider over all 25 results. We keep 3. Those searches can't use an index, so they get slower as the directory grows.
Nothing waits on the response, so it's not a latency problem, but it is recurring database work on every page load.
Limiting the query to 3 instead of 25 and skipping the action providers would cover most of it. Caching per user for a few minutes on top would make repeat loads free, and slightly stale recent contacts are fine.
There was a problem hiding this comment.
Agree, I think even 5 minutes caching would be fine here
| }) | ||
|
|
||
| // immediate: load header avatars on mount and whenever the team filter changes | ||
| watch(selectedTeam, loadPreviewAvatars, { immediate: true }) |
There was a problem hiding this comment.
@kristian-zendato I didn't mean to add a second watcher here. This one has the same problem: it fires immediately, then always fires a second time we set selectedTeam above. Also since we can't predict in which order the responses arrive, this can lead to race conditions.
storage.getItem is synchronous, so perhaps we can seed the ref instead of assigning in onMounted:
const storedTeam = storage.getItem('core:contacts:team')
const selectedTeam = ref<string>(storedTeam ? JSON.parse(storedTeam) : '$_all_$')Then drop the if (team) block. One request, correct team, all cases.
3 participants
Summary
Makes the People (contacts) header menu more inviting by showing a stack of up to 3 overlapping avatars of recent contacts on the menu trigger, instead of only the contacts icon.
GET /contactsmenu/preview-avatarswhich returns the first 3 entries from the same unfiltered contacts list as the menu (Manager::getEntrieswith an empty filter), with optional team filtering matching the existing index endpoint.NcAvatarstack when at least 2 are available; otherwise keeps the existing contacts icon.width: fit-content) so the stack does not overlap notifications/profile and remains fully clickable.Checklist
3. to review, feature component)stable32)AI (if applicable)