feat(core): app menu polish for NC34

Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>

Author: pringelmann

core/src/components/AppMenu.vue

@@ -49,13 +49,14 @@
 			v-if="currentApp"
 			class="app-menu__current-app"
 			variant="tertiary-no-background"
-			:aria-label="t('core', 'Open apps menu')"
+			:aria-label="currentAppLabel"
 			aria-haspopup="menu"
 			:aria-expanded="opened ? 'true' : 'false'"
 			@click="onTriggerClick('currentApp')">
 			<template #icon>
 				<img
 					class="app-menu__current-app-icon"
+					:class="{ 'app-menu__current-app-icon--settings': currentApp.type === 'settings' }"
 					:src="currentApp.icon"
 					alt=""
 					aria-hidden="true">
@@ -82,6 +83,9 @@ import IconDotsGrid from 'vue-material-design-icons/DotsGrid.vue'
 import AppItem from './AppItem.vue'
 import logger from '../logger.js'
 
+// Settings IDs that represent actions, not navigable pages.
+const SETTINGS_ACTION_IDS = new Set(['logout'])
+
 export default defineComponent({
 	name: 'AppMenu',
 
@@ -103,8 +107,12 @@ export default defineComponent({
 
 	data() {
 		const appList = loadState<INavigationEntry[]>('core', 'apps', [])
+		// Record<id, entry>, not an array: PHP ships getAll('settings') without
+		// array_values(). Matches AccountMenu.vue's usage.
+		const settingsList = loadState<Record<string, INavigationEntry>>('core', 'settingsNavEntries', {})
 		return {
 			appList,
+			settingsList,
 			isAdmin: getCurrentUser()?.isAdmin ?? false,
 			// Roving tabindex: only this tile has tabindex=0; arrow keys move it.
 			focusedIndex: 0,
@@ -146,7 +154,18 @@ export default defineComponent({
 
 	computed: {
 		currentApp(): INavigationEntry | undefined {
+			// Fall back to the active settings entry on admin pages where no
+			// app is active.
 			return this.appList.find((app) => app.active)
+				?? Object.values(this.settingsList).find((entry) => entry.active && !SETTINGS_ACTION_IDS.has(entry.id))
+		},
+
+		// aria-label overrides the inner span text, so the section name
+		// has to be duplicated here for screen readers.
+		currentAppLabel(): string {
+			return this.currentApp
+				? t('core', 'Open apps menu, currently in {app}', { app: this.currentApp.name })
+				: t('core', 'Open apps menu')
 		},
 
 		// Stable-ordered list that focusedIndex indexes into. The trailing
@@ -374,6 +393,12 @@ export default defineComponent({
 		// Theme-aware inversion + vertical alpha fade via --header-menu-icon-mask.
 		filter: var(--background-image-invert-if-bright);
 		mask: var(--header-menu-icon-mask);
+
+		// Settings icons ship dark (designed for the white settings sidebar);
+		// force-white so they read against the themed header.
+		&--settings {
+			filter: brightness(0) invert(1);
+		}
 	}
 
 	&__current-app-name {
@@ -386,16 +411,20 @@ export default defineComponent({
 	&__popover {
 		max-width: calc(100vw - var(--default-grid-baseline) * 4);
 		background-color: var(--color-main-background);
+		// Padding on the popover (not the grid) so the top-row hover sits
+		// concentrically inside the rounded corners.
+		padding: calc(var(--default-grid-baseline) * 3) calc(var(--default-grid-baseline) * 2);
 	}
 
 	&__grid {
 		--app-item-col-width: 69px;
 		--app-item-row-height: 64px;
 		--app-menu-rows-visible: 6;
-		padding: calc(var(--default-grid-baseline) * 3) calc(var(--default-grid-baseline) * 2);
 		display: grid;
 		grid-template-columns: repeat(4, var(--app-item-col-width));
 		grid-auto-rows: minmax(var(--app-item-row-height), max-content);
+		// + baseline * 5: peek-row hint so users see that content continues
+		// below the fold.
 		max-height: calc(var(--app-item-row-height) * var(--app-menu-rows-visible) + var(--default-grid-baseline) * 5);
 		overflow-y: auto;
 

core/src/tests/components/AppMenu.spec.ts

@@ -165,4 +165,64 @@ describe('core: AppMenu', () => {
 		const currentApp = wrapper.get('.app-menu__current-app').element
 		expect(wrapper.vm.returnFocusTarget()).toBe(currentApp)
 	})
+
+	it('falls back to the active settings entry when no app is active', () => {
+		// Mimics being on /settings/admin/* where the active entry is registered
+		// as type=settings (NavigationManager) and excluded from the `apps` list.
+		initialState.loadState.mockImplementation((_a: string, key: string, fallback: unknown) => {
+			if (key === 'apps') {
+				return [makeApp({ id: 'files', name: 'Files', active: false })]
+			}
+			if (key === 'settingsNavEntries') {
+				// Object keyed by entry id — matches PHP's serialization shape
+				// (TemplateLayout ships the filtered associative array as-is).
+				return {
+					admin_settings: makeApp({
+						id: 'admin_settings',
+						name: 'Administration settings',
+						type: 'settings',
+						href: '/settings/admin/overview',
+						icon: '/settings/img/admin.svg',
+						active: true,
+					}),
+				}
+			}
+			return fallback
+		})
+		const wrapper = mount(AppMenu, { attachTo: document.body })
+		expect(wrapper.find('.app-menu__current-app').exists()).toBe(true)
+		expect(wrapper.find('.app-menu__current-app-name').text()).toBe('Administration settings')
+	})
+
+	it('prefers the active app over a settings entry when both are marked active', () => {
+		initialState.loadState.mockImplementation((_a: string, key: string, fallback: unknown) => {
+			if (key === 'apps') {
+				return [makeApp({ id: 'files', name: 'Files', active: true })]
+			}
+			if (key === 'settingsNavEntries') {
+				return { admin_settings: makeApp({ id: 'admin_settings', name: 'Administration settings', type: 'settings', active: true }) }
+			}
+			return fallback
+		})
+		const wrapper = mount(AppMenu, { attachTo: document.body })
+		expect(wrapper.find('.app-menu__current-app-name').text()).toBe('Files')
+	})
+
+	it('does not render the current-app button when only the logout entry is active', () => {
+		// Defensive: logout is an action, not a page, so it should never be the
+		// "current section" even though it carries type=settings. NavigationManager
+		// today never marks it active, but a future regression shouldn't leak a
+		// "Log out" label into the header.
+		initialState.loadState.mockImplementation((_a: string, key: string, fallback: unknown) => {
+			if (key === 'apps') {
+				return [makeApp({ id: 'files', name: 'Files', active: false })]
+			}
+			if (key === 'settingsNavEntries') {
+				return { logout: makeApp({ id: 'logout', name: 'Log out', type: 'settings', href: '/logout', active: true }) }
+			}
+			return fallback
+		})
+		const wrapper = mount(AppMenu, { attachTo: document.body })
+		expect(wrapper.find('.app-menu__current-app').exists()).toBe(false)
+	})
 })