Merge pull request #41122 from nextcloud/enh/a11y/separate-profile-entry

enh(a11y): Separate profile and user status user menu entries

apps/user_status/src/UserStatus.vue

@@ -21,28 +21,13 @@
 
 <template>
 	<component :is="inline ? 'div' : 'li'">
-		<!-- User Menu Entries -->
-		<div v-if="!inline" class="user-status-menu-item">
-			<!-- Username display -->
-			<a class="user-status-menu-item__header"
-				:href="profilePageLink"
-				@click.exact="loadProfilePage">
-				<div class="user-status-menu-item__header-content">
-					<div class="user-status-menu-item__header-content-displayname">{{ displayName }}</div>
-					<div v-if="!loadingProfilePage" class="user-status-menu-item__header-content-placeholder" />
-					<div v-else class="icon-loading-small" />
-				</div>
-				<div v-if="profileEnabled">
-					{{ t('user_status', 'View profile') }}
-				</div>
-			</a>
-
-			<!-- User Status = Status modal toggle -->
-			<button class="user-status-menu-item__toggle" @click.stop="openModal">
-				<span aria-hidden="true" :class="statusIcon" class="user-status-icon" />
-				{{ visibleMessage }}
-			</button>
-		</div>
+		<!-- User Status = Status modal toggle -->
+		<button v-if="!inline"
+			class="user-status-menu-item"
+			@click.stop="openModal">
+			<span aria-hidden="true" :class="statusIcon" class="user-status-icon" />
+			{{ visibleMessage }}
+		</button>
 
 		<!-- Dashboard Status -->
 		<NcButton v-else
@@ -60,9 +45,6 @@
 </template>
 
 <script>
-import { generateUrl } from '@nextcloud/router'
-import { getCurrentUser } from '@nextcloud/auth'
-import { loadState } from '@nextcloud/initial-state'
 import { subscribe, unsubscribe } from '@nextcloud/event-bus'
 import NcButton from '@nextcloud/vue/dist/Components/NcButton.js'
 import debounce from 'debounce'
@@ -70,8 +52,6 @@ import debounce from 'debounce'
 import { sendHeartbeat } from './services/heartbeatService.js'
 import OnlineStatusMixin from './mixins/OnlineStatusMixin.js'
 
-const { profileEnabled } = loadState('user_status', 'profileEnabled', false)
-
 export default {
 	name: 'UserStatus',
 
@@ -95,41 +75,19 @@ export default {
 
 	data() {
 		return {
-			displayName: getCurrentUser().displayName,
 			heartbeatInterval: null,
 			isAway: false,
 			isModalOpen: false,
-			loadingProfilePage: false,
 			mouseMoveListener: null,
-			profileEnabled,
 			setAwayTimeout: null,
 		}
 	},
-	computed: {
-		/**
-		 * The profile page link
-		 *
-		 * @return {string | undefined}
-		 */
-		profilePageLink() {
-			if (this.profileEnabled) {
-				return generateUrl('/u/{userId}', { userId: getCurrentUser().uid })
-			}
-			// Since an anchor element is used rather than a button,
-			// this hack removes href if the profile is disabled so that disabling pointer-events is not needed to prevent a click from opening a page
-			// and to allow the hover event for styling
-			return undefined
-		},
-	},
 
 	/**
 	 * Loads the current user's status from initial state
 	 * and stores it in Vuex
 	 */
 	mounted() {
-		subscribe('settings:display-name:updated', this.handleDisplayNameUpdate)
-		subscribe('settings:profile-enabled:updated', this.handleProfileEnabledUpdate)
-
 		this.$store.dispatch('loadStatusFromInitialState')
 
 		if (OC.config.session_keepalive) {
@@ -166,28 +124,12 @@ export default {
 	 * Some housekeeping before destroying the component
 	 */
 	beforeDestroy() {
-		unsubscribe('settings:display-name:updated', this.handleDisplayNameUpdate)
-		unsubscribe('settings:profile-enabled:updated', this.handleProfileEnabledUpdate)
 		window.removeEventListener('mouseMove', this.mouseMoveListener)
 		clearInterval(this.heartbeatInterval)
 		unsubscribe('user_status:status.updated', this.handleUserStatusUpdated)
 	},
 
 	methods: {
-		handleDisplayNameUpdate(displayName) {
-			this.displayName = displayName
-		},
-
-		handleProfileEnabledUpdate(profileEnabled) {
-			this.profileEnabled = profileEnabled
-		},
-
-		loadProfilePage() {
-			if (this.profileEnabled) {
-				this.loadingProfilePage = true
-			}
-		},
-
 		/**
 		 * Opens the modal to set a custom status
 		 */
@@ -234,75 +176,27 @@ export default {
 
 <style lang="scss" scoped>
 .user-status-menu-item {
-	&__header {
-		display: flex !important;
-		flex-direction: column !important;
-		width: auto !important;
-		height: 44px * 1.5 !important;
-		padding: 10px 12px 5px 12px !important;
-		align-items: flex-start !important;
-		color: var(--color-main-text) !important;
+	width: auto;
+	min-width: 44px;
+	height: 44px;
+	margin: 0;
+	border: 0;
+	border-radius: var(--border-radius-pill);
+	background-color: var(--color-main-background-blur);
+	font-size: inherit;
+	font-weight: normal;
 
-		&:not([href]) {
-			height: var(--header-menu-item-height) !important;
-			color: var(--color-text-maxcontrast) !important;
-			cursor: default !important;
+	-webkit-backdrop-filter: var(--background-blur);
+	backdrop-filter: var(--background-blur);
 
-			& * {
-				cursor: default !important;
-			}
-
-			&:hover {
-				background-color: transparent !important;
-			}
-		}
-
-		&-content {
-			display: inline-flex !important;
-			font-weight: bold !important;
-			gap: 0 10px !important;
-			width: auto;
-
-			&-displayname {
-				width: auto;
-			}
-
-			&-placeholder {
-				width: 16px !important;
-				height: 24px !important;
-				margin-right: 10px !important;
-				visibility: hidden !important;
-			}
-		}
-
-		span {
-			color: var(--color-text-maxcontrast) !important;
-		}
+	&:active,
+	&:hover,
+	&:focus-visible {
+		background-color: var(--color-background-hover);
 	}
-
-	&__toggle {
-		width: auto;
-		min-width: 44px;
-		height: 44px;
-		margin: 0;
-		border: 0;
-		border-radius: var(--border-radius-pill);
-		background-color: var(--color-main-background-blur);
-		font-size: inherit;
-		font-weight: normal;
-
-		-webkit-backdrop-filter: var(--background-blur);
-		backdrop-filter: var(--background-blur);
-
-		&:active,
-		&:hover,
-		&:focus {
-			background-color: var(--color-background-hover);
-		}
-		&:focus-visible {
-			box-shadow: 0 0 0 4px var(--color-main-background) !important;
-			outline: 2px solid var(--color-main-text) !important;
-		}
+	&:focus-visible {
+		box-shadow: 0 0 0 4px var(--color-main-background) !important;
+		outline: 2px solid var(--color-main-text) !important;
 	}
 }
 

core/Controller/ProfilePageController.php

@@ -40,6 +40,7 @@ use OCP\IUserSession;
 use OCP\Share\IManager as IShareManager;
 use OCP\UserStatus\IManager as IUserStatusManager;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\INavigationManager;
 
 #[IgnoreOpenAPI]
 class ProfilePageController extends Controller {
@@ -52,6 +53,7 @@ class ProfilePageController extends Controller {
 		private IUserManager $userManager,
 		private IUserSession $userSession,
 		private IUserStatusManager $userStatusManager,
+		private INavigationManager $navigationManager,
 		private IEventDispatcher $eventDispatcher,
 	) {
 		parent::__construct($appName, $request);
@@ -104,6 +106,8 @@ class ProfilePageController extends Controller {
 			$this->profileManager->getProfileFields($targetUser, $visitingUser),
 		);
 
+		$this->navigationManager->setActiveEntry('profile');
+
 		$this->eventDispatcher->dispatchTyped(new BeforeTemplateRenderedEvent($targetUserId));
 
 		\OCP\Util::addScript('core', 'profile');

core/src/components/UserMenu/ProfileUserMenuEntry.vue

@@ -0,0 +1,140 @@
+<!--
+  - @copyright 2023 Christopher Ng <chrng8@gmail.com>
+  -
+  - @author Christopher Ng <chrng8@gmail.com>
+  -
+  - @license AGPL-3.0-or-later
+  -
+  - This program is free software: you can redistribute it and/or modify
+  - it under the terms of the GNU Affero General Public License as
+  - published by the Free Software Foundation, either version 3 of the
+  - License, or (at your option) any later version.
+  -
+  - This program is distributed in the hope that it will be useful,
+  - but WITHOUT ANY WARRANTY; without even the implied warranty of
+  - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+  - GNU Affero General Public License for more details.
+  -
+  - You should have received a copy of the GNU Affero General Public License
+  - along with this program. If not, see <http://www.gnu.org/licenses/>.
+  -
+-->
+
+<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>
+</template>
+
+<script>
+import { loadState } from '@nextcloud/initial-state'
+import { getCurrentUser } from '@nextcloud/auth'
+import { subscribe, unsubscribe } from '@nextcloud/event-bus'
+
+import NcLoadingIcon from '@nextcloud/vue/dist/Components/NcLoadingIcon.js'
+
+const { profileEnabled } = loadState('user_status', 'profileEnabled', false)
+
+export default {
+	name: 'ProfileUserMenuEntry',
+
+	components: {
+		NcLoadingIcon,
+	},
+
+	props: {
+		id: {
+			type: String,
+			required: true,
+		},
+		name: {
+			type: String,
+			required: true,
+		},
+		href: {
+			type: String,
+			required: true,
+		},
+		active: {
+			type: Boolean,
+			required: true,
+		},
+	},
+
+	data() {
+		return {
+			profileEnabled,
+			displayName: getCurrentUser().displayName,
+			loading: false,
+		}
+	},
+
+	mounted() {
+		subscribe('settings:profile-enabled:updated', this.handleProfileEnabledUpdate)
+		subscribe('settings:display-name:updated', this.handleDisplayNameUpdate)
+	},
+
+	beforeDestroy() {
+		unsubscribe('settings:profile-enabled:updated', this.handleProfileEnabledUpdate)
+		unsubscribe('settings:display-name:updated', this.handleDisplayNameUpdate)
+	},
+
+	methods: {
+		handleClick() {
+			if (this.profileEnabled) {
+				this.loading = true
+			}
+		},
+
+		handleProfileEnabledUpdate(profileEnabled) {
+			this.profileEnabled = profileEnabled
+		},
+
+		handleDisplayNameUpdate(displayName) {
+			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>

core/src/views/UserMenu.vue

@@ -35,7 +35,11 @@
 				:preloaded-user-status="userStatus" />
 		</template>
 		<ul>
-			<UserMenuEntry v-for="entry in settingsNavEntries"
+			<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"
@@ -58,6 +62,7 @@ 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'
@@ -75,8 +80,9 @@ import logger from '../logger.js'
  * @property {string} classes - Classes for custom styling
  */
 
-/** @type {SettingNavEntry[]} */
+/** @type {Record<string, SettingNavEntry>} */
 const settingsNavEntries = loadState('core', 'settingsNavEntries', [])
+const { profile: profileEntry, ...otherEntries } = settingsNavEntries
 
 const translateStatus = (status) => {
 	const statusMap = Object.fromEntries(
@@ -95,12 +101,14 @@ export default {
 	components: {
 		NcAvatar,
 		NcHeaderMenu,
+		ProfileUserMenuEntry,
 		UserMenuEntry,
 	},
 
 	data() {
 		return {
-			settingsNavEntries,
+			profileEntry,
+			otherEntries,
 			displayName: getCurrentUser()?.displayName,
 			userId: getCurrentUser()?.uid,
 			isLoadingUserStatus: true,

lib/private/NavigationManager.php

@@ -219,6 +219,18 @@ class NavigationManager implements INavigationManager {
 		}
 
 		if ($this->userSession->isLoggedIn()) {
+			// Profile
+			$this->add([
+				'type' => 'settings',
+				'id' => 'profile',
+				'order' => 1,
+				'href' => $this->urlGenerator->linkToRoute(
+					'core.ProfilePage.index',
+					['targetUserId' => $this->userSession->getUser()->getUID()],
+				),
+				'name' => $l->t('View profile'),
+			]);
+
 			// Accessibility settings
 			if ($this->appManager->isEnabledForUser('theming', $this->userSession->getUser())) {
 				$this->add([
@@ -230,6 +242,7 @@ class NavigationManager implements INavigationManager {
 					'icon' => $this->urlGenerator->imagePath('theming', 'accessibility-dark.svg'),
 				]);
 			}
+
 			if ($this->isAdmin()) {
 				// App management
 				$this->add([

tests/lib/NavigationManagerTest.php

@@ -276,6 +276,17 @@ class NavigationManagerTest extends TestCase {
 			]
 		];
 		$defaults = [
+			'profile' => [
+				'type' => 'settings',
+				'id' => 'profile',
+				'order' => 1,
+				'href' => '/apps/test/',
+				'name' => 'View profile',
+				'icon' => '',
+				'active' => false,
+				'classes' => '',
+				'unread' => 0,
+			],
 			'accessibility_settings' => [
 				'type' => 'settings',
 				'id' => 'accessibility_settings',
@@ -339,6 +350,7 @@ class NavigationManagerTest extends TestCase {
 		return [
 			'minimalistic' => [
 				array_merge(
+					['profile' => $defaults['profile']],
 					['accessibility_settings' => $defaults['accessibility_settings']],
 					['settings' => $defaults['settings']],
 					['test' => [
@@ -365,6 +377,7 @@ class NavigationManagerTest extends TestCase {
 			],
 			'minimalistic-settings' => [
 				array_merge(
+					['profile' => $defaults['profile']],
 					['accessibility_settings' => $defaults['accessibility_settings']],
 					['settings' => $defaults['settings']],
 					['test' => [
@@ -388,6 +401,7 @@ class NavigationManagerTest extends TestCase {
 			],
 			'with-multiple' => [
 				array_merge(
+					['profile' => $defaults['profile']],
 					['accessibility_settings' => $defaults['accessibility_settings']],
 					['settings' => $defaults['settings']],
 					['test' => [
@@ -429,6 +443,7 @@ class NavigationManagerTest extends TestCase {
 			],
 			'admin' => [
 				array_merge(
+					['profile' => $defaults['profile']],
 					$adminSettings,
 					$apps,
 					['test' => [
@@ -456,6 +471,7 @@ class NavigationManagerTest extends TestCase {
 			],
 			'no name' => [
 				array_merge(
+					['profile' => $defaults['profile']],
 					$adminSettings,
 					$apps,
 					['logout' => $defaults['logout']]