From 674805c9948460df49592d187f5bb9a4cb8f4b56 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Mon, 29 Jul 2024 14:05:45 +0200
Subject: [PATCH] fix(core): Migrate UserMenu / AccountMenu to NcListItem

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 .../AccountMenuEntry.vue}                     |  58 ++--
 .../AccountMenuProfileEntry.vue}              |  85 +++---
 core/src/components/UserMenu.js               |   4 +-
 core/src/views/AccountMenu.vue                | 238 ++++++++++++++++
 core/src/views/UserMenu.vue                   | 259 ------------------
 cypress/e2e/settings/access-levels.cy.ts      |   8 +-
 6 files changed, 313 insertions(+), 339 deletions(-)
 rename core/src/components/{UserMenu/UserMenuEntry.vue => AccountMenu/AccountMenuEntry.vue} (51%)
 rename core/src/components/{UserMenu/ProfileUserMenuEntry.vue => AccountMenu/AccountMenuProfileEntry.vue} (52%)
 create mode 100644 core/src/views/AccountMenu.vue
 delete mode 100644 core/src/views/UserMenu.vue

diff --git a/core/src/components/UserMenu/UserMenuEntry.vue b/core/src/components/AccountMenu/AccountMenuEntry.vue
similarity index 51%
rename from core/src/components/UserMenu/UserMenuEntry.vue
rename to core/src/components/AccountMenu/AccountMenuEntry.vue
index 8e9a2d4494c..47ce884fc2b 100644
--- a/core/src/components/UserMenu/UserMenuEntry.vue
+++ b/core/src/components/AccountMenu/AccountMenuEntry.vue
@@ -4,36 +4,39 @@
 -->
 
 <template>
-	<li :id="id"
-		class="menu-entry">
-		<a v-if="href"
-			:href="href"
-			:class="{ active }"
-			@click.exact="handleClick">
-			<NcLoadingIcon v-if="loading"
-				class="menu-entry__loading-icon"
-				:size="18" />
-			<img v-else :src="cachedIcon" alt="">
-			{{ name }}
-		</a>
-		<button v-else>
-			<img :src="cachedIcon" alt="">
-			{{ name }}
-		</button>
-	</li>
+	<NcListItem :id="href ? undefined : id"
+		:anchor-id="id"
+		:active="active"
+		class="account-menu-entry"
+		compact
+		:href="href"
+		:name="name"
+		target="_self">
+		<template #icon>
+			<img class="account-menu-entry__icon"
+				:class="{ 'account-menu-entry__icon--active': active }"
+				:src="iconSource"
+				alt="">
+		</template>
+		<template v-if="loading" #indicator>
+			<NcLoadingIcon />
+		</template>
+	</NcListItem>
 </template>
 
 <script>
 import { loadState } from '@nextcloud/initial-state'
 
+import NcListItem from '@nextcloud/vue/dist/Components/NcListItem.js'
 import NcLoadingIcon from '@nextcloud/vue/dist/Components/NcLoadingIcon.js'
 
 const versionHash = loadState('core', 'versionHash', '')
 
 export default {
-	name: 'UserMenuEntry',
+	name: 'AccountMenuEntry',
 
 	components: {
+		NcListItem,
 		NcLoadingIcon,
 	},
 
@@ -67,7 +70,7 @@ export default {
 	},
 
 	computed: {
-		cachedIcon() {
+		iconSource() {
 			return `${this.icon}?v=${versionHash}`
 		},
 	},
@@ -81,9 +84,20 @@ export default {
 </script>
 
 <style lang="scss" scoped>
-.menu-entry {
-	&__loading-icon {
-		margin-right: 8px;
+.account-menu-entry {
+	&__icon {
+		height: 20px;
+		width: 20px;
+		margin: calc((var(--default-clickable-area) - 20px) / 2); // 20px icon size
+		filter: var(--background-invert-if-dark);
+
+		&--active {
+			filter: var(--primary-invert-if-dark);
+		}
+	}
+
+	:deep(.list-item-content__main) {
+		width: fit-content;
 	}
 }
 </style>
diff --git a/core/src/components/UserMenu/ProfileUserMenuEntry.vue b/core/src/components/AccountMenu/AccountMenuProfileEntry.vue
similarity index 52%
rename from core/src/components/UserMenu/ProfileUserMenuEntry.vue
rename to core/src/components/AccountMenu/AccountMenuProfileEntry.vue
index 1e203c85b27..853c22986ce 100644
--- a/core/src/components/UserMenu/ProfileUserMenuEntry.vue
+++ b/core/src/components/AccountMenu/AccountMenuProfileEntry.vue
@@ -4,38 +4,38 @@
 -->
 
 <template>
-	<li :id="id"
-		class="menu-entry">
-		<component :is="profileEnabled ? 'a' : 'span'"
-			class="menu-entry__wrapper"
-			:class="{
-				active,
-				'menu-entry__wrapper--link': profileEnabled,
-			}"
-			:href="profileEnabled ? href : undefined"
-			@click.exact="handleClick">
-			<span class="menu-entry__content">
-				<span class="menu-entry__displayname">{{ displayName }}</span>
-				<NcLoadingIcon v-if="loading" :size="18" />
-			</span>
-			<span v-if="profileEnabled">{{ name }}</span>
-		</component>
-	</li>
+	<NcListItem :id="profileEnabled ? undefined : id"
+		:anchor-id="id"
+		:active="active"
+		compact
+		:href="profileEnabled ? href : undefined"
+		:name="displayName"
+		target="_self">
+		<template v-if="profileEnabled" #subname>
+			{{ name }}
+		</template>
+		<template v-if="loading" #indicator>
+			<NcLoadingIcon />
+		</template>
+	</NcListItem>
 </template>
 
-<script>
+<script lang="ts">
 import { loadState } from '@nextcloud/initial-state'
 import { getCurrentUser } from '@nextcloud/auth'
 import { subscribe, unsubscribe } from '@nextcloud/event-bus'
+import { defineComponent } from 'vue'
 
+import NcListItem from '@nextcloud/vue/dist/Components/NcListItem.js'
 import NcLoadingIcon from '@nextcloud/vue/dist/Components/NcLoadingIcon.js'
 
-const { profileEnabled } = loadState('user_status', 'profileEnabled', false)
+const { profileEnabled } = loadState('user_status', 'profileEnabled', { profileEnabled: false })
 
-export default {
-	name: 'ProfileUserMenuEntry',
+export default defineComponent({
+	name: 'AccountMenuProfileEntry',
 
 	components: {
+		NcListItem,
 		NcLoadingIcon,
 	},
 
@@ -58,10 +58,15 @@ export default {
 		},
 	},
 
-	data() {
+	setup() {
 		return {
 			profileEnabled,
-			displayName: getCurrentUser().displayName,
+			displayName: getCurrentUser()!.displayName,
+		}
+	},
+
+	data() {
+		return {
 			loading: false,
 		}
 	},
@@ -83,41 +88,13 @@ export default {
 			}
 		},
 
-		handleProfileEnabledUpdate(profileEnabled) {
+		handleProfileEnabledUpdate(profileEnabled: boolean) {
 			this.profileEnabled = profileEnabled
 		},
 
-		handleDisplayNameUpdate(displayName) {
+		handleDisplayNameUpdate(displayName: string) {
 			this.displayName = displayName
 		},
 	},
-}
+})
 </script>
-
-<style lang="scss" scoped>
-.menu-entry {
-	&__wrapper {
-		box-sizing: border-box;
-		display: inline-flex;
-		flex-direction: column;
-		align-items: flex-start !important;
-		padding: 10px 12px 5px 12px !important;
-		height: var(--header-menu-item-height);
-		color: var(--color-text-maxcontrast);
-
-		&--link {
-			height: calc(var(--header-menu-item-height) * 1.5) !important;
-			color: var(--color-main-text);
-		}
-	}
-
-	&__content {
-		display: inline-flex;
-		gap: 0 10px;
-	}
-
-	&__displayname {
-		font-weight: bold;
-	}
-}
-</style>
diff --git a/core/src/components/UserMenu.js b/core/src/components/UserMenu.js
index 94fa8e62ab5..37895bb6166 100644
--- a/core/src/components/UserMenu.js
+++ b/core/src/components/UserMenu.js
@@ -5,7 +5,7 @@
 
 import Vue from 'vue'
 
-import UserMenu from '../views/UserMenu.vue'
+import AccountMenu from '../views/AccountMenu.vue'
 
 export const setUp = () => {
 	const mountPoint = document.getElementById('user-menu')
@@ -13,7 +13,7 @@ export const setUp = () => {
 		// eslint-disable-next-line no-new
 		new Vue({
 			el: mountPoint,
-			render: h => h(UserMenu),
+			render: h => h(AccountMenu),
 		})
 	}
 }
diff --git a/core/src/views/AccountMenu.vue b/core/src/views/AccountMenu.vue
new file mode 100644
index 00000000000..1eef398e087
--- /dev/null
+++ b/core/src/views/AccountMenu.vue
@@ -0,0 +1,238 @@
+<!--
+ - SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
+ - SPDX-License-Identifier: AGPL-3.0-or-later
+-->
+<template>
+	<NcHeaderMenu id="user-menu"
+		class="account-menu"
+		is-nav
+		:aria-label="t('core', 'Settings menu')"
+		:description="avatarDescription">
+		<template #trigger>
+			<!-- The `key` is a hack as NcAvatar does not handle updating the preloaded status on show status change -->
+			<NcAvatar :key="String(showUserStatus)"
+				class="account-menu__avatar"
+				disable-menu
+				disable-tooltip
+				:show-user-status="showUserStatus"
+				:user="currentUserId"
+				:preloaded-user-status="userStatus" />
+		</template>
+		<ul class="account-menu__list">
+			<AccountMenuProfileEntry :id="profileEntry.id"
+				:name="profileEntry.name"
+				:href="profileEntry.href"
+				:active="profileEntry.active" />
+			<AccountMenuEntry v-for="entry in otherEntries"
+				:id="entry.id"
+				:key="entry.id"
+				:name="entry.name"
+				:href="entry.href"
+				:active="entry.active"
+				:icon="entry.icon" />
+		</ul>
+	</NcHeaderMenu>
+</template>
+
+<script lang="ts">
+import { getCurrentUser } from '@nextcloud/auth'
+import { emit, subscribe } from '@nextcloud/event-bus'
+import { loadState } from '@nextcloud/initial-state'
+import { t } from '@nextcloud/l10n'
+import { generateOcsUrl } from '@nextcloud/router'
+import { getCapabilities } from '@nextcloud/capabilities'
+import { defineComponent } from 'vue'
+import { getAllStatusOptions } from '../../../apps/user_status/src/services/statusOptionsService.js'
+
+import axios from '@nextcloud/axios'
+import logger from '../logger.js'
+
+import NcAvatar from '@nextcloud/vue/dist/Components/NcAvatar.js'
+import NcHeaderMenu from '@nextcloud/vue/dist/Components/NcHeaderMenu.js'
+import AccountMenuProfileEntry from '../components/AccountMenu/AccountMenuProfileEntry.vue'
+import AccountMenuEntry from '../components/AccountMenu/AccountMenuEntry.vue'
+
+interface ISettingsNavigationEntry {
+	/**
+	 * id of the entry, used as HTML ID, for example, "settings"
+	 */
+	id: string
+	/**
+	 * Label of the entry, for example, "Personal Settings"
+	 */
+	name: string
+	/**
+	 * Icon of the entry, for example, "/apps/settings/img/personal.svg"
+	 */
+	icon: string
+	/**
+	 * Type of the entry
+	 */
+	type: 'settings'|'link'|'guest'
+	/**
+	 * Link of the entry, for example, "/settings/user"
+	 */
+	href: string
+	/**
+	 * Whether the entry is active
+	 */
+	active: boolean
+	/**
+	 * Order of the entry
+	 */
+	order: number
+	/**
+	 * Number of unread pf this items
+	 */
+	unread: number
+	/**
+	 * Classes for custom styling
+	 */
+	classes: string
+}
+
+const USER_DEFINABLE_STATUSES = getAllStatusOptions()
+
+export default defineComponent({
+	name: 'AccountMenu',
+
+	components: {
+		AccountMenuEntry,
+		AccountMenuProfileEntry,
+		NcAvatar,
+		NcHeaderMenu,
+	},
+
+	setup() {
+		const settingsNavEntries = loadState<Record<string, ISettingsNavigationEntry>>('core', 'settingsNavEntries', {})
+		const { profile: profileEntry, ...otherEntries } = settingsNavEntries
+
+		return {
+			currentDisplayName: getCurrentUser()?.displayName ?? getCurrentUser()!.uid,
+			currentUserId: getCurrentUser()!.uid,
+
+			profileEntry,
+			otherEntries,
+
+			t,
+		}
+	},
+
+	data() {
+		return {
+			showUserStatus: false,
+			userStatus: {
+				status: null,
+				icon: null,
+				message: null,
+			},
+		}
+	},
+
+	computed: {
+		translatedUserStatus() {
+			return {
+				...this.userStatus,
+				status: this.translateStatus(this.userStatus.status),
+			}
+		},
+
+		avatarDescription() {
+			const description = [
+				t('core', 'Avatar of {displayName}', { displayName: this.currentDisplayName }),
+				...Object.values(this.translatedUserStatus).filter(Boolean),
+			].join(' — ')
+			return description
+		},
+	},
+
+	async created() {
+		if (!getCapabilities()?.user_status?.enabled) {
+			return
+		}
+
+		const url = generateOcsUrl('/apps/user_status/api/v1/user_status')
+		try {
+			const response = await axios.get(url)
+			const { status, icon, message } = response.data.ocs.data
+			this.userStatus = { status, icon, message }
+		} catch (e) {
+			logger.error('Failed to load user status')
+		}
+		this.showUserStatus = true
+	},
+
+	mounted() {
+		subscribe('user_status:status.updated', this.handleUserStatusUpdated)
+		emit('core:user-menu:mounted')
+	},
+
+	methods: {
+		handleUserStatusUpdated(state) {
+			if (this.currentUserId === state.userId) {
+				this.userStatus = {
+					status: state.status,
+					icon: state.icon,
+					message: state.message,
+				}
+			}
+		},
+
+		translateStatus(status) {
+			const statusMap = Object.fromEntries(
+				USER_DEFINABLE_STATUSES.map(({ type, label }) => [type, label]),
+			)
+			if (statusMap[status]) {
+				return statusMap[status]
+			}
+			return status
+		},
+	},
+})
+</script>
+
+<style lang="scss" scoped>
+:deep(#header-menu-user-menu) {
+	padding: 0 !important;
+}
+
+.account-menu {
+	:deep(button) {
+		// Normally header menus are slightly translucent when not active
+		// this is generally ok but for the avatar this is weird so fix the opacity
+		opacity: 1 !important;
+
+		// The avatar is just the "icon" of the button
+		// So we add the focus-visible manually
+		&:focus-visible {
+			.account-menu__avatar {
+				border: var(--border-width-input-focused) solid var(--color-background-plain-text);
+			}
+		}
+	}
+
+	// Ensure we do not wast space, as the header menu sets a default width of 350px
+	:deep(.header-menu__content) {
+		width: fit-content !important;
+	}
+
+	&__avatar {
+		&:hover {
+			// Add hover styles similar to the focus-visible style
+			border: var(--border-width-input-focused) solid var(--color-background-plain-text);
+		}
+	}
+
+	&__list {
+		display: inline-flex;
+		flex-direction: column;
+		gap: var(--default-grid-baseline);
+
+		> :deep(li) {
+			box-sizing: border-box;
+			// basically "fit-content"
+			flex: 0 1;
+		}
+	}
+}
+</style>
diff --git a/core/src/views/UserMenu.vue b/core/src/views/UserMenu.vue
deleted file mode 100644
index 1351743dd05..00000000000
--- a/core/src/views/UserMenu.vue
+++ /dev/null
@@ -1,259 +0,0 @@
-<!--
- - SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
- - SPDX-License-Identifier: AGPL-3.0-or-later
--->
-<template>
-	<NcHeaderMenu id="user-menu"
-		class="user-menu"
-		is-nav
-		:aria-label="t('core', 'Settings menu')"
-		:description="avatarDescription">
-		<template #trigger>
-			<NcAvatar v-if="!isLoadingUserStatus"
-				class="user-menu__avatar"
-				:disable-menu="true"
-				:disable-tooltip="true"
-				:user="userId"
-				:preloaded-user-status="userStatus" />
-		</template>
-		<ul>
-			<ProfileUserMenuEntry :id="profileEntry.id"
-				:name="profileEntry.name"
-				:href="profileEntry.href"
-				:active="profileEntry.active" />
-			<UserMenuEntry v-for="entry in otherEntries"
-				:id="entry.id"
-				:key="entry.id"
-				:name="entry.name"
-				:href="entry.href"
-				:active="entry.active"
-				:icon="entry.icon" />
-		</ul>
-	</NcHeaderMenu>
-</template>
-
-<script>
-import axios from '@nextcloud/axios'
-import { emit, subscribe } from '@nextcloud/event-bus'
-import { loadState } from '@nextcloud/initial-state'
-import { generateOcsUrl } from '@nextcloud/router'
-import { getCurrentUser } from '@nextcloud/auth'
-import { getCapabilities } from '@nextcloud/capabilities'
-
-import NcAvatar from '@nextcloud/vue/dist/Components/NcAvatar.js'
-import NcHeaderMenu from '@nextcloud/vue/dist/Components/NcHeaderMenu.js'
-
-import { getAllStatusOptions } from '../../../apps/user_status/src/services/statusOptionsService.js'
-import ProfileUserMenuEntry from '../components/UserMenu/ProfileUserMenuEntry.vue'
-import UserMenuEntry from '../components/UserMenu/UserMenuEntry.vue'
-
-import logger from '../logger.js'
-
-/**
- * @typedef SettingNavEntry
- * @property {string} id - id of the entry, used as HTML ID, for example, "settings"
- * @property {string} name - Label of the entry, for example, "Personal Settings"
- * @property {string} icon - Icon of the entry, for example, "/apps/settings/img/personal.svg"
- * @property {'settings'|'link'|'guest'} type - Type of the entry
- * @property {string} href - Link of the entry, for example, "/settings/user"
- * @property {boolean} active - Whether the entry is active
- * @property {number} order - Order of the entry
- * @property {number} unread - Number of unread pf this items
- * @property {string} classes - Classes for custom styling
- */
-
-/** @type {Record<string, SettingNavEntry>} */
-const settingsNavEntries = loadState('core', 'settingsNavEntries', [])
-const { profile: profileEntry, ...otherEntries } = settingsNavEntries
-
-const translateStatus = (status) => {
-	const statusMap = Object.fromEntries(
-		getAllStatusOptions()
-			.map(({ type, label }) => [type, label]),
-	)
-	if (statusMap[status]) {
-		return statusMap[status]
-	}
-	return status
-}
-
-export default {
-	name: 'UserMenu',
-
-	components: {
-		NcAvatar,
-		NcHeaderMenu,
-		ProfileUserMenuEntry,
-		UserMenuEntry,
-	},
-
-	data() {
-		return {
-			profileEntry,
-			otherEntries,
-			displayName: getCurrentUser()?.displayName,
-			userId: getCurrentUser()?.uid,
-			isLoadingUserStatus: true,
-			userStatus: {
-				status: null,
-				icon: null,
-				message: null,
-			},
-		}
-	},
-
-	computed: {
-		translatedUserStatus() {
-			return {
-				...this.userStatus,
-				status: translateStatus(this.userStatus.status),
-			}
-		},
-
-		avatarDescription() {
-			const description = [
-				t('core', 'Avatar of {displayName}', { displayName: this.displayName }),
-				...Object.values(this.translatedUserStatus).filter(Boolean),
-			].join(' — ')
-			return description
-		},
-	},
-
-	async created() {
-		if (!getCapabilities()?.user_status?.enabled) {
-			this.isLoadingUserStatus = false
-			return
-		}
-
-		const url = generateOcsUrl('/apps/user_status/api/v1/user_status')
-		try {
-			const response = await axios.get(url)
-			const { status, icon, message } = response.data.ocs.data
-			this.userStatus = { status, icon, message }
-		} catch (e) {
-			logger.error('Failed to load user status')
-		}
-		this.isLoadingUserStatus = false
-	},
-
-	mounted() {
-		subscribe('user_status:status.updated', this.handleUserStatusUpdated)
-		emit('core:user-menu:mounted')
-	},
-
-	methods: {
-		handleUserStatusUpdated(state) {
-			if (this.userId === state.userId) {
-				this.userStatus = {
-					status: state.status,
-					icon: state.icon,
-					message: state.message,
-				}
-			}
-		},
-	},
-}
-</script>
-
-<style lang="scss" scoped>
-.user-menu {
-	&:deep {
-		.header-menu {
-			&__trigger {
-				opacity: 1 !important;
-				&:focus-visible {
-					.user-menu__avatar {
-						border: 2px solid var(--color-primary-element);
-					}
-				}
-			}
-
-			&__carret {
-				display: none !important;
-			}
-
-			&__content {
-				width: fit-content !important;
-			}
-		}
-	}
-
-	&__avatar {
-		&:active,
-		&:focus,
-		&:hover {
-			border: 2px solid var(--color-primary-element-text);
-		}
-	}
-
-	ul {
-		display: flex;
-		flex-direction: column;
-		gap: 2px;
-
-		&:deep {
-			li {
-				a,
-				button {
-					border-radius: 6px;
-					display: inline-flex;
-					align-items: center;
-					height: var(--header-menu-item-height);
-					color: var(--color-main-text);
-					padding: 10px 8px;
-					box-sizing: border-box;
-					white-space: nowrap;
-					position: relative;
-					width: 100%;
-
-					&:hover {
-						background-color: var(--color-background-hover);
-					}
-
-					&:focus-visible {
-						background-color: var(--color-background-hover) !important;
-						box-shadow: inset 0 0 0 2px var(--color-primary-element) !important;
-						outline: none !important;
-					}
-
-					&:active:not(:focus-visible),
-					&.active:not(:focus-visible) {
-						background-color: var(--color-primary-element);
-						color: var(--color-primary-element-text);
-
-						img {
-							filter: var(--primary-invert-if-dark);
-						}
-					}
-
-					span {
-						padding-bottom: 0;
-						white-space: nowrap;
-						overflow: hidden;
-						text-overflow: ellipsis;
-						max-width: 210px;
-					}
-
-					img {
-						width: 16px;
-						height: 16px;
-						margin-right: 10px;
-					}
-
-					img {
-						filter: var(--background-invert-if-dark);
-					}
-				}
-
-				// Override global button styles
-				button {
-					background-color: transparent;
-					border: none;
-					font-weight: normal;
-					margin: 0;
-				}
-			}
-		}
-	}
-}
-</style>
diff --git a/cypress/e2e/settings/access-levels.cy.ts b/cypress/e2e/settings/access-levels.cy.ts
index 80cea4eb1c7..4bf0cbc1832 100644
--- a/cypress/e2e/settings/access-levels.cy.ts
+++ b/cypress/e2e/settings/access-levels.cy.ts
@@ -23,7 +23,9 @@ describe('Settings: Ensure only administrator can see the administration setting
 		// I open the settings menu
 		getNextcloudUserMenuToggle().click()
 		// I navigate to the settings panel
-		getNextcloudUserMenu().find('#settings a').click()
+		getNextcloudUserMenu()
+			.findByRole('link', { name: /settings/i })
+			.click()
 		cy.url().should('match', /\/settings\/user$/)
 
 		cy.get('#app-navigation').should('be.visible').within(() => {
@@ -45,7 +47,9 @@ describe('Settings: Ensure only administrator can see the administration setting
 		// I open the settings menu
 		getNextcloudUserMenuToggle().click()
 		// I navigate to the settings panel
-		getNextcloudUserMenu().find('#settings a').click()
+		getNextcloudUserMenu()
+			.findByRole('link', { name: /Personal settings/i })
+			.click()
 		cy.url().should('match', /\/settings\/user$/)
 
 		cy.get('#app-navigation').should('be.visible').within(() => {