Skip to content

Commit 98c0a5a

Browse files
jancborchardtjancborchardt
committed
feat(core): unify the app menu triggers into one full-height hit area
Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Jan C. Borchardt <925062+jancborchardt@users.noreply.github.com>
1 parent e68d903 commit 98c0a5a

1 file changed

Lines changed: 129 additions & 96 deletions

File tree

‎core/src/components/AppMenu.vue‎

Lines changed: 129 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -5,73 +5,79 @@
55

66
<template>
77
<nav class="app-menu" :aria-label="t('core', 'Applications')">
8-
<NcPopover
9-
ref="popover"
10-
:shown="opened"
11-
:triggers="[]"
12-
placement="bottom-start"
13-
:skidding="popoverSkidding"
14-
:set-return-focus="returnFocusTarget"
15-
popover-base-class="app-menu__popover-base"
16-
popup-role="menu"
17-
@update:shown="opened = $event">
18-
<template #trigger>
19-
<NcButton
20-
class="app-menu__waffle"
21-
variant="tertiary-no-background"
22-
:aria-label="t('core', 'Open apps menu')"
23-
aria-haspopup="menu"
24-
:aria-expanded="opened ? 'true' : 'false'"
25-
@click="onTriggerClick('waffle')">
26-
<template #icon>
27-
<IconDotsGrid :size="20" />
28-
</template>
29-
</NcButton>
30-
</template>
31-
32-
<div
33-
class="app-menu__popover"
34-
role="menu"
35-
:aria-label="t('core', 'Apps')">
36-
<div ref="grid" class="app-menu__grid" @keydown="onGridKeydown">
37-
<AppItem
38-
v-for="(item, i) in gridItems"
39-
:key="item.id"
40-
ref="items"
41-
:app="item"
42-
:outlined="item.id === 'more-apps' || item.id === 'app-store'"
43-
:new-tab="item.id === 'app-store'"
44-
:tabindex="i === focusedIndex ? 0 : -1" />
8+
<!-- One wrapper so both triggers act as a single control, sharing one
9+
highlight. On narrow screens only the waffle shows. -->
10+
<div
11+
class="app-menu__trigger"
12+
:class="{ 'app-menu__trigger--open': opened }">
13+
<NcPopover
14+
ref="popover"
15+
:shown="opened"
16+
:triggers="[]"
17+
placement="bottom-start"
18+
:skidding="popoverSkidding"
19+
:set-return-focus="returnFocusTarget"
20+
popover-base-class="app-menu__popover-base"
21+
popup-role="menu"
22+
@update:shown="opened = $event">
23+
<template #trigger>
24+
<NcButton
25+
class="app-menu__waffle"
26+
variant="tertiary-no-background"
27+
:aria-label="t('core', 'Open apps menu')"
28+
aria-haspopup="menu"
29+
:aria-expanded="opened ? 'true' : 'false'"
30+
@click="onTriggerClick('waffle')">
31+
<template #icon>
32+
<IconDotsGrid :size="20" />
33+
</template>
34+
</NcButton>
35+
</template>
36+
37+
<div
38+
class="app-menu__popover"
39+
role="menu"
40+
:aria-label="t('core', 'Apps')">
41+
<div ref="grid" class="app-menu__grid" @keydown="onGridKeydown">
42+
<AppItem
43+
v-for="(item, i) in gridItems"
44+
:key="item.id"
45+
ref="items"
46+
:app="item"
47+
:outlined="item.id === 'more-apps' || item.id === 'app-store'"
48+
:new-tab="item.id === 'app-store'"
49+
:tabindex="i === focusedIndex ? 0 : -1" />
50+
</div>
4551
</div>
46-
</div>
47-
</NcPopover>
48-
<NcButton
49-
v-if="currentApp"
50-
class="app-menu__current-app"
51-
variant="tertiary-no-background"
52-
:aria-label="currentAppLabel"
53-
aria-haspopup="menu"
54-
:aria-expanded="opened ? 'true' : 'false'"
55-
@click="onTriggerClick('currentApp')">
56-
<template #icon>
57-
<!-- Settings sub-sections share one generic cog. An inline MDI icon
58-
inherits the button's currentColor (--color-background-plain-text),
59-
so it stays legible on both bright and dark headers without a filter. -->
60-
<IconCog
61-
v-if="currentApp.type === 'settings'"
62-
class="app-menu__current-app-cog"
63-
:size="20" />
64-
<img
65-
v-else
66-
class="app-menu__current-app-icon"
67-
:src="currentApp.icon"
68-
alt=""
69-
aria-hidden="true">
70-
</template>
71-
<span class="app-menu__current-app-name">
72-
{{ displayName }}
73-
</span>
74-
</NcButton>
52+
</NcPopover>
53+
<NcButton
54+
v-if="currentApp"
55+
class="app-menu__current-app"
56+
variant="tertiary-no-background"
57+
:aria-label="currentAppLabel"
58+
aria-haspopup="menu"
59+
:aria-expanded="opened ? 'true' : 'false'"
60+
@click="onTriggerClick('currentApp')">
61+
<template #icon>
62+
<!-- Settings sub-sections share one generic cog. An inline MDI icon
63+
inherits the button's currentColor (--color-background-plain-text),
64+
so it stays legible on both bright and dark headers without a filter. -->
65+
<IconCog
66+
v-if="currentApp.type === 'settings'"
67+
class="app-menu__current-app-cog"
68+
:size="20" />
69+
<img
70+
v-else
71+
class="app-menu__current-app-icon"
72+
:src="currentApp.icon"
73+
alt=""
74+
aria-hidden="true">
75+
</template>
76+
<span class="app-menu__current-app-name">
77+
{{ displayName }}
78+
</span>
79+
</NcButton>
80+
</div>
7581
</nav>
7682
</template>
7783

@@ -397,56 +403,83 @@ export default defineComponent({
397403
display: flex;
398404
align-items: center;
399405
400-
&__waffle {
401-
// NcButton's tertiary-no-background variant uses --color-main-text,
402-
// which is dark on light themes. The header sits on the theme primary
403-
// background, so override to use the matching plain-text color.
404-
--color-main-text: var(--color-background-plain-text);
405-
color: var(--color-background-plain-text);
406-
407-
// Class merges onto NcButton's root <button>; style directly, no :deep().
408-
// !important: v8 NcButton's legacy bundle sets focus-visible
409-
// outline/box-shadow with !important, same as the current-app :active rule.
410-
&:hover:not(:disabled) {
411-
background-color: rgba(0, 0, 0, 0.1) !important;
406+
// Wrapper for both triggers: full header height for the click area, with one
407+
// shared highlight spanning the waffle and the current app.
408+
&__trigger {
409+
position: relative;
410+
display: flex;
411+
align-items: center;
412+
height: var(--header-height);
413+
// Own stacking context so the highlight can sit behind the buttons.
414+
isolation: isolate;
415+
416+
// The shared highlight, inset from top/bottom so it stays smaller than the
417+
// header. inset-inline: 0 spans both triggers; collapses to the waffle when
418+
// the current-app button is hidden.
419+
&::before {
420+
content: '';
421+
position: absolute;
422+
inset-block: calc((var(--header-height) - var(--default-clickable-area)) / 2);
423+
inset-inline: 0;
424+
border-radius: var(--border-radius-element);
425+
// Behind the buttons (whose backgrounds stay transparent).
426+
z-index: -1;
427+
pointer-events: none;
412428
}
413429
414-
&:active:not(:disabled) {
415-
background-color: rgba(0, 0, 0, 0.15) !important;
430+
// Translucent black: --color-background-hover has too little contrast on the
431+
// header tint. Keep the highlight while the menu is open.
432+
&:hover::before,
433+
&--open::before {
434+
background-color: rgba(0, 0, 0, 0.1);
416435
}
417436
418-
&:focus-visible {
419-
background-color: rgba(0, 0, 0, 0.1) !important;
420-
outline: none !important;
421-
box-shadow: inset 0 0 0 2px var(--color-background-plain-text) !important;
437+
&:active::before {
438+
background-color: rgba(0, 0, 0, 0.15);
422439
}
423440
}
424441
442+
&__waffle,
425443
&__current-app {
444+
// Full header height for the click area; the highlight is on __trigger, so
445+
// the buttons stay transparent. !important beats NcButton's scoped rules.
446+
height: var(--header-height) !important;
447+
// Anchor the per-button focus ring below to the button, not __trigger.
448+
position: relative;
449+
426450
// NcButton's tertiary-no-background variant uses --color-main-text,
427451
// which is dark on light themes. The header sits on the theme primary
428452
// background, so override to use the matching plain-text color.
429453
--color-main-text: var(--color-background-plain-text);
430454
color: var(--color-background-plain-text);
431455
432-
// !important: v8 NcButton's legacy bundle sets focus-visible
433-
// outline/box-shadow with !important. Same translucent-black hover/
434-
// active overlays as the waffle: --color-background-hover collapses
435-
// contrast against the theme-primary header tint.
436-
&:hover:not(:disabled) {
437-
background-color: rgba(0, 0, 0, 0.1) !important;
456+
// Hide NcButton's own hover/active fill so only the __trigger highlight
457+
// shows. The extra .button-vue makes this win over NcButton's rule.
458+
&.button-vue:hover:not(:disabled),
459+
&.button-vue:active:not(:disabled) {
460+
background-color: transparent !important;
438461
}
439462
440-
&:active:not(:disabled) {
441-
background-color: rgba(0, 0, 0, 0.15) !important;
463+
// Per-button keyboard focus ring, matched to the highlight pill. Hide
464+
// NcButton's own ring (outline + halo); the extra .button-vue makes our
465+
// override win over it.
466+
&.button-vue:focus-visible {
467+
outline: none !important;
468+
box-shadow: none !important;
442469
}
443470
444-
&:focus-visible {
445-
background-color: rgba(0, 0, 0, 0.1) !important;
446-
outline: none !important;
447-
box-shadow: inset 0 0 0 2px var(--color-background-plain-text) !important;
471+
&.button-vue:focus-visible::before {
472+
content: '';
473+
position: absolute;
474+
inset-block: calc((var(--header-height) - var(--default-clickable-area)) / 2);
475+
inset-inline: 0;
476+
border-radius: var(--border-radius-element);
477+
box-shadow: inset 0 0 0 2px var(--color-background-plain-text);
478+
pointer-events: none;
448479
}
480+
}
449481
482+
&__current-app {
450483
// Lets the inner label shrink to its max-width and ellipsize instead of
451484
// pushing the button wider than the inline-flex text slot.
452485
:deep(.button-vue__text) {

0 commit comments

Comments
 (0)