From 91f78936c94e2124d14defead1fa4ce14e7ee734 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Mon, 5 Aug 2024 13:08:16 +0200
Subject: [PATCH] fix: Ensure app overflow menu is rendered centered

1. Fix app menu overflow button to be centered
2. Ensure the overflow calculation always just removed one element (incl. refactor to use `@vueuse`)
3. Ensure we use 1.5 line height for app menu, otherwise some languages look squashed under the app icon

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 core/src/components/AppMenu.vue      | 79 ++++++++++++++--------------
 core/src/components/AppMenuEntry.vue | 10 ++--
 2 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/core/src/components/AppMenu.vue b/core/src/components/AppMenu.vue
index 33e1a194f3ca6..f982776827e0e 100644
--- a/core/src/components/AppMenu.vue
+++ b/core/src/components/AppMenu.vue
@@ -4,7 +4,8 @@
 -->
 
 <template>
-	<nav class="app-menu"
+	<nav ref="appMenu"
+		class="app-menu"
 		:aria-label="t('core', 'Applications menu')">
 		<ul class="app-menu__list">
 			<AppMenuEntry v-for="app in mainAppList"
@@ -29,7 +30,8 @@ import type { INavigationEntry } from '../types/navigation'
 import { subscribe, unsubscribe } from '@nextcloud/event-bus'
 import { loadState } from '@nextcloud/initial-state'
 import { n, t } from '@nextcloud/l10n'
-import { defineComponent } from 'vue'
+import { useElementSize } from '@vueuse/core'
+import { defineComponent, ref } from 'vue'
 
 import AppMenuEntry from './AppMenuEntry.vue'
 import NcActions from '@nextcloud/vue/dist/Components/NcActions.js'
@@ -46,40 +48,47 @@ export default defineComponent({
 	},
 
 	setup() {
+		const appMenu = ref()
+		const { width: appMenuWidth } = useElementSize(appMenu)
 		return {
 			t,
 			n,
+			appMenu,
+			appMenuWidth,
 		}
 	},
 
 	data() {
 		const appList = loadState<INavigationEntry[]>('core', 'apps', [])
-
 		return {
 			appList,
-			appLimit: 0,
-			observer: null as ResizeObserver | null,
 		}
 	},
 
 	computed: {
+		appLimit() {
+			const maxApps = Math.floor(this.appMenuWidth / 50)
+			if (maxApps < this.appList.length) {
+				// Ensure there is space for the overflow menu
+				return Math.max(maxApps - 1, 0)
+			}
+			return maxApps
+		},
+
 		mainAppList() {
 			return this.appList.slice(0, this.appLimit)
 		},
+
 		popoverAppList() {
 			return this.appList.slice(this.appLimit)
 		},
 	},
 
 	mounted() {
-		this.observer = new ResizeObserver(this.resize)
-		this.observer.observe(this.$el)
-		this.resize()
 		subscribe('nextcloud:app-menu.refresh', this.setApps)
 	},
 
 	beforeDestroy() {
-		this.observer!.disconnect()
 		unsubscribe('nextcloud:app-menu.refresh', this.setApps)
 	},
 
@@ -96,54 +105,44 @@ export default defineComponent({
 		setApps({ apps }: { apps: INavigationEntry[]}) {
 			this.appList = apps
 		},
-
-		resize() {
-			const availableWidth = (this.$el as HTMLElement).offsetWidth
-			let appCount = Math.floor(availableWidth / 50) - 1
-			const popoverAppCount = this.appList.length - appCount
-			if (popoverAppCount === 1) {
-				appCount--
-			}
-			if (appCount < 1) {
-				appCount = 0
-			}
-			this.appLimit = appCount
-		},
 	},
 })
 </script>
 
 <style scoped lang="scss">
 .app-menu {
-	width: 100%;
 	display: flex;
-	flex-shrink: 1;
-	flex-wrap: wrap;
+	flex: 1 1;
+	width: 0;
 
 	&__list {
 		display: flex;
 		flex-wrap: nowrap;
 	}
 
-	// Adjust the overflow NcActions styles as they are directly rendered on the background
-	&__overflow :deep(.button-vue--vue-tertiary) {
-		opacity: .7;
-		margin: 3px;
-		filter: var(--background-image-invert-if-bright);
+	&__overflow {
+		margin-block: auto;
 
-		/* Remove all background and align text color if not expanded */
-		&:not([aria-expanded="true"]) {
-			color: var(--color-background-plain-text);
+		// Adjust the overflow NcActions styles as they are directly rendered on the background
+		:deep(.button-vue--vue-tertiary) {
+			opacity: .7;
+			margin: 3px;
+			filter: var(--background-image-invert-if-bright);
 
-			&:hover {
-				opacity: 1;
-				background-color: transparent !important;
+			/* Remove all background and align text color if not expanded */
+			&:not([aria-expanded="true"]) {
+				color: var(--color-background-plain-text);
+
+				&:hover {
+					opacity: 1;
+					background-color: transparent !important;
+				}
 			}
-		}
 
-		&:focus-visible {
-			opacity: 1;
-			outline: none !important;
+			&:focus-visible {
+				opacity: 1;
+				outline: none !important;
+			}
 		}
 	}
 
diff --git a/core/src/components/AppMenuEntry.vue b/core/src/components/AppMenuEntry.vue
index aada0c63930a5..ead86759e4a07 100644
--- a/core/src/components/AppMenuEntry.vue
+++ b/core/src/components/AppMenuEntry.vue
@@ -33,6 +33,7 @@ defineProps<{
 
 <style scoped lang="scss">
 .app-menu-entry {
+	--app-menu-entry-font-size: 12px;
 	width: var(--header-height);
 	height: var(--header-height);
 	position: relative;
@@ -54,8 +55,7 @@ defineProps<{
 	&__label {
 		opacity: 0;
 		position: absolute;
-		font-size: 12px;
-		line-height: 1.25;
+		font-size: var(--app-menu-entry-font-size);
 		// this is shown directly on the background
 		color: var(--color-background-plain-text);
 		text-align: center;
@@ -71,6 +71,10 @@ defineProps<{
 		letter-spacing: -0.5px;
 	}
 
+	&__icon {
+		font-size: var(--app-menu-entry-font-size);
+	}
+
 	&--active {
 		// When hover or focus, show the label and make it bolder than the other entries
 		.app-menu-entry__label {
@@ -117,7 +121,7 @@ defineProps<{
 .app-menu__list:focus-within {
 	// Move icon up so that the name does not overflow the icon
 	.app-menu-entry__icon {
-		margin-block-end: calc(1.5 * 12px); // font size of label * line height
+		margin-block-end: 1lh;
 	}
 
 	// Make the label visible