-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
feat: show recent people on contact icon #62930
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,16 +15,21 @@ | |
| use OCP\AppFramework\Http\Attribute\NoAdminRequired; | ||
| use OCP\AppFramework\Http\JSONResponse; | ||
| use OCP\Contacts\ContactsMenu\IEntry; | ||
| use OCP\ICacheFactory; | ||
| use OCP\IRequest; | ||
| use OCP\IUserSession; | ||
| use OCP\Teams\ITeamManager; | ||
|
|
||
| class ContactsMenuController extends Controller { | ||
| private const PREVIEW_AVATARS_LIMIT = 3; | ||
| private const PREVIEW_AVATARS_CACHE_TTL = 300; | ||
|
|
||
| public function __construct( | ||
| IRequest $request, | ||
| private IUserSession $userSession, | ||
| private Manager $manager, | ||
| private ITeamManager $teamManager, | ||
| private ICacheFactory $cacheFactory, | ||
| ) { | ||
| parent::__construct('core', $request); | ||
| } | ||
|
|
@@ -70,4 +75,43 @@ public function findOne(int $shareType, string $shareWith) { | |
| public function getTeams(): array { | ||
| return $this->teamManager->getTeamsForUser($this->userSession->getUser()->getUID()); | ||
| } | ||
|
|
||
| /** | ||
| * Top contacts for the People menu header avatar stack (max 3). | ||
| * Uses a lightweight query (limited results, no action providers) and | ||
| * caches per user for a few minutes. | ||
| * | ||
| * @return list<array> | ||
| * @throws Exception | ||
| */ | ||
| #[NoAdminRequired] | ||
| #[FrontpageRoute(verb: 'GET', url: '/contactsmenu/preview-avatars')] | ||
| public function previewAvatars(?string $teamId = null): array { | ||
| $user = $this->userSession->getUser(); | ||
| if ($user === null) { | ||
| return []; | ||
| } | ||
|
|
||
| $cache = $this->cacheFactory->createDistributed('contactsmenu-preview'); | ||
| $cacheKey = $user->getUID(); | ||
| $cached = $cache->get($cacheKey); | ||
| if (!is_array($cached)) { | ||
|
Comment on lines
+95
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if the caching is worth it here. The cache TTL is 5min, which means most likely the cache will never get utilized that much. Most users will not flip between the apps that frequently, they will click on mail work there for 10min then maybe change over the calendar extra. Power users will just open each app in a separate window, so not sure this will really help any. @kesselb @DerDreschner what do you think? |
||
| $entries = $this->manager->getPreviewEntries($user, self::PREVIEW_AVATARS_LIMIT); | ||
| $cached = array_map( | ||
| static fn (IEntry $entry): array => $entry->jsonSerialize(), | ||
| $entries, | ||
| ); | ||
| $cache->set($cacheKey, $cached, self::PREVIEW_AVATARS_CACHE_TTL); | ||
| } | ||
|
|
||
| if ($teamId !== null && $teamId !== '') { | ||
| $memberIds = $this->teamManager->getMembersOfTeam($teamId, $user->getUID()); | ||
| $cached = array_filter( | ||
| $cached, | ||
| static fn (array $entry): bool => array_key_exists($entry['uid'] ?? '', $memberIds) | ||
| ); | ||
| } | ||
|
|
||
| return array_values(array_slice($cached, 0, self::PREVIEW_AVATARS_LIMIT)); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ import debounce from 'debounce' | |
| import { computed, nextTick, onMounted, ref, watch } from 'vue' | ||
| import NcActionButton from '@nextcloud/vue/components/NcActionButton' | ||
| import NcActions from '@nextcloud/vue/components/NcActions' | ||
| import NcAvatar from '@nextcloud/vue/components/NcAvatar' | ||
| import NcButton from '@nextcloud/vue/components/NcButton' | ||
| import NcEmptyContent from '@nextcloud/vue/components/NcEmptyContent' | ||
| import NcHeaderMenu from '@nextcloud/vue/components/NcHeaderMenu' | ||
|
|
@@ -23,6 +24,12 @@ import NcTextField from '@nextcloud/vue/components/NcTextField' | |
| import ContactMenuEntry from '../components/ContactsMenu/ContactMenuEntry.vue' | ||
| import logger from '../logger.js' | ||
|
|
||
| interface IPreviewUser { | ||
| uid: string | ||
| fullName: string | ||
| isUser: boolean | ||
| } | ||
|
|
||
|
Comment on lines
+27
to
+32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interfaces should be extracted in to a separate types file "src/types" |
||
| const storage = getBuilder('core:contacts') | ||
| .persist(true) | ||
| .clearOnLogout(true) | ||
|
|
@@ -42,15 +49,13 @@ const hasError = ref(false) | |
| const searchTerm = ref('') | ||
|
|
||
| const teams = ref<ITeam[]>([]) | ||
| const selectedTeam = ref<string>('$_all_$') | ||
| const storedTeam = storage.getItem('core:contacts:team') | ||
| const selectedTeam = ref<string>(storedTeam ? JSON.parse(storedTeam) : '$_all_$') | ||
| const selectedTeamName = computed(() => teams.value.find((t) => t.teamId === selectedTeam.value)?.displayName) | ||
| const previewUsers = ref<IPreviewUser[]>([]) | ||
| const showAvatarStack = computed(() => previewUsers.value.length >= 2) | ||
|
|
||
| onMounted(async () => { | ||
| const team = storage.getItem('core:contacts:team') | ||
| if (team) { | ||
| selectedTeam.value = JSON.parse(team) | ||
| } | ||
|
|
||
| if (userTeams.length === 0) { | ||
| try { | ||
| const { data } = await axios.get<ITeam[]>(generateUrl('/contactsmenu/teams')) | ||
|
|
@@ -65,8 +70,29 @@ onMounted(async () => { | |
| watch(selectedTeam, () => { | ||
| storage.setItem('core:contacts:team', JSON.stringify(selectedTeam.value)) | ||
| getContacts(searchTerm.value) | ||
| loadPreviewAvatars() | ||
| }) | ||
|
|
||
| /** | ||
| * Load avatars for the People menu header trigger | ||
| */ | ||
| async function loadPreviewAvatars() { | ||
| try { | ||
| const { data } = await axios.get<IPreviewUser[]>(generateUrl('/contactsmenu/preview-avatars'), { | ||
| params: { | ||
| teamId: selectedTeam.value !== '$_all_$' ? selectedTeam.value : undefined, | ||
| }, | ||
| }) | ||
| previewUsers.value = data | ||
| } catch (error) { | ||
| logger.error('could not load preview avatars', { error }) | ||
| previewUsers.value = [] | ||
| } | ||
| } | ||
|
|
||
| // Seeded selectedTeam above so this runs once on mount with the correct team | ||
| loadPreviewAvatars() | ||
|
|
||
| /** | ||
| * Load contacts when opening the menu | ||
| */ | ||
|
|
@@ -145,11 +171,32 @@ const userTeams: ITeam[] = [] | |
| <NcHeaderMenu | ||
| id="contactsmenu" | ||
| class="contactsmenu" | ||
| :class="{ 'contactsmenu--avatar-stack': showAvatarStack }" | ||
| :aria-label="t('core', 'Search contacts')" | ||
| exclude-click-outside-selectors=".v-popper__popper" | ||
| @open="onOpened"> | ||
| <template #trigger> | ||
| <NcIconSvgWrapper class="contactsmenu__trigger-icon" :path="mdiContacts" /> | ||
| <span | ||
| v-if="showAvatarStack" | ||
| class="contactsmenu__trigger-avatars" | ||
| aria-hidden="true"> | ||
| <NcAvatar | ||
| v-for="(previewUser, index) in previewUsers" | ||
| :key="previewUser.isUser ? previewUser.uid : `${previewUser.fullName}-${index}`" | ||
| class="contactsmenu__trigger-avatars__avatar" | ||
| :style="{ zIndex: previewUsers.length - index }" | ||
| :user="previewUser.isUser ? previewUser.uid : undefined" | ||
| :is-no-user="!previewUser.isUser" | ||
| :display-name="previewUser.fullName" | ||
| :size="32" | ||
| disable-menu | ||
| disable-tooltip | ||
| hide-status /> | ||
| </span> | ||
| <NcIconSvgWrapper | ||
| v-else | ||
| class="contactsmenu__trigger-icon" | ||
| :path="mdiContacts" /> | ||
| </template> | ||
| <div class="contactsmenu__menu"> | ||
| <div class="contactsmenu__menu__search-container"> | ||
|
|
@@ -242,12 +289,67 @@ const userTeams: ITeam[] = [] | |
|
|
||
| <style lang="scss" scoped> | ||
| .contactsmenu { | ||
| overflow-y: hidden; | ||
| margin-inline-end: calc(2 * var(--default-grid-baseline)); | ||
|
|
||
| :deep(.header-menu__trigger) { | ||
| // NcHeaderMenu applies --header-menu-icon-mask (vertical alpha fade) to | ||
| // .button-vue__icon:not(:has(svg)). Avatars need the full face visible. | ||
| .button-vue__icon:has(.contactsmenu__trigger-avatars) { | ||
| mask: none !important; | ||
| } | ||
| } | ||
|
|
||
| &--avatar-stack { | ||
| width: fit-content !important; | ||
| min-width: var(--header-height); | ||
| overflow: visible; | ||
| flex-shrink: 0; | ||
|
|
||
| :deep(.header-menu__trigger) { | ||
| width: fit-content !important; | ||
| min-width: var(--header-height); | ||
| max-width: none; | ||
| overflow: visible !important; | ||
| padding-inline: var(--default-grid-baseline); | ||
|
|
||
| .button-vue__wrapper { | ||
| width: auto; | ||
| justify-content: center; | ||
| } | ||
|
|
||
| .button-vue__icon { | ||
| width: auto !important; | ||
| min-width: 0; | ||
| max-width: none; | ||
| height: auto; | ||
| min-height: 0; | ||
| overflow: visible; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| &__trigger-icon { | ||
| color: var(--color-background-plain-text) !important; | ||
| } | ||
|
|
||
| &__trigger-avatars { | ||
| display: flex; | ||
| align-items: center; | ||
| pointer-events: none; | ||
|
|
||
| &__avatar { | ||
| box-sizing: content-box; | ||
| flex-shrink: 0; | ||
| --contactsmenu-avatar-outline: var(--border-width-input) solid color-mix(in srgb, var(--color-background-plain-text), transparent 75%); | ||
| outline: var(--contactsmenu-avatar-outline); | ||
| margin-inline-start: -12px; | ||
|
|
||
| &:first-child { | ||
| margin-inline-start: 0; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| &__menu { | ||
| display: flex; | ||
| flex-direction: column; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,23 @@ public function getEntries(IUser $user, ?string $filter): array { | |
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * Lightweight recent contacts for the People menu header avatar stack. | ||
| * Limits the store query and skips action providers. | ||
| * | ||
| * @return IEntry[] | ||
| * @throws Exception | ||
| */ | ||
| public function getPreviewEntries(IUser $user, int $limit = 3): array { | ||
| $limit = max(0, $limit); | ||
| if ($limit === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| $entries = $this->store->getContacts($user, '', $limit); | ||
| return array_slice($this->sortEntries($entries), 0, $limit); | ||
|
Comment on lines
+63
to
+64
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the idea to get any 3 contacts? Or just the last 3 recent contacts?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is to get last 3 recent contacts. |
||
| } | ||
|
|
||
| /** | ||
| * @throws Exception | ||
| */ | ||
|
|
||
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.
Please add rate limiting
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.
UserRateLimit(limit: 60, period: 120)would be enough for it?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.
Tbh, I don't know what a reasonable threshold might be... I can't see a regular user reloading the page every 2s?
Make the span 5min, and the limit 30 times... even that is over kill i think