Skip to content

Commit 192eb9c

Browse files
authored
Merge pull request #120 from fleetbase/fix/smart-nav-menu-shortcut-bugs
fix(smart-nav-menu/dropdown): fix shortcut duplication and full-card click area
2 parents 4a55526 + b84e19f commit 192eb9c

4 files changed

Lines changed: 120 additions & 152 deletions

File tree

addon/components/layout/header/smart-nav-menu/dropdown.hbs

Lines changed: 77 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
Layout::Header::SmartNavMenu::Dropdown
33
Phase 2: multi-column card grid with search filter.
44
5-
Shortcuts are expanded as independent sibling items in the grid (AWS-style).
6-
The JS getter `expandedItems` interleaves parent MenuItems and their
7-
shortcut objects so each appears as its own flat card.
5+
Shortcuts are already registered as first-class items in the universe
6+
registry by menu-service.registerHeaderMenuItem(), so @items already
7+
contains both parent extension items and their shortcut siblings.
8+
No client-side expansion is needed.
89
910
Args:
1011
@items - Array of MenuItem objects to display
@@ -15,9 +16,13 @@
1516
@onQuickPin - Action to pin an item directly from the dropdown
1617
@atPinnedLimit - Boolean: true when the bar is full (pin button hidden)
1718
18-
NOTE: Route-based cards use <LinkToExternal> with no click handler.
19-
The dropdown is closed via the routeDidChange listener in smart-nav-menu.js
20-
which sets isMoreOpen = false on every route transition.
19+
Click-area pattern:
20+
Each card is wrapped in a `snm-dropdown-card-wrap` div (position: relative).
21+
The card itself is the <LinkToExternal> (or <a> for onClick items), making
22+
the entire card surface clickable. The pin button is a sibling of the card
23+
inside the wrapper, absolutely positioned in the top-right corner so it
24+
sits above the card link and receives its own clicks without violating the
25+
no-nested-interactive rule.
2126
}}
2227
<div
2328
class="snm-dropdown snm-dropdown--wide"
@@ -38,7 +43,7 @@
3843
</button>
3944
</div>
4045

41-
{{! ── Search bar (own row) ────────────────────────────────────────────── }}
46+
{{! ── Search bar ──────────────────────────────────────────────────────── }}
4247
<div class="snm-dropdown-search-bar">
4348
<span class="snm-dropdown-search-icon" aria-hidden="true">
4449
<FaIcon @icon="magnifying-glass" @size="xs" />
@@ -76,59 +81,58 @@
7681
{{else}}
7782
{{#each this.filteredItems as |item|}}
7883
{{#if item._isShortcut}}
79-
{{! ── Shortcut sibling card (AWS-style flat item) ─────── }}
80-
<div class="snm-dropdown-card" role="presentation">
81-
<div class="snm-dropdown-card-header">
82-
<LinkToExternal
83-
@route={{item.route}}
84-
id={{concat (dasherize (or item.route item.id "sc")) "-dropdown-card"}}
85-
class="snm-dropdown-card-link"
86-
title={{item.title}}
87-
>
84+
{{! ── Shortcut card ───────────────────────────────────── }}
85+
<div class="snm-dropdown-card-wrap">
86+
<LinkToExternal
87+
@route={{item.route}}
88+
id={{concat (dasherize (or item.route item.id "sc")) "-dropdown-card"}}
89+
class="snm-dropdown-card"
90+
title={{item.title}}
91+
>
92+
<div class="snm-dropdown-card-header">
8893
<span class="snm-dropdown-card-icon">
8994
{{#if item.iconComponent}}
9095
{{component (lazy-engine-component item.iconComponent) options=item.iconComponentOptions}}
9196
{{else}}
9297
<FaIcon @icon={{or item.icon "circle-dot"}} @prefix={{item.iconPrefix}} @size="sm" />
9398
{{/if}}
9499
</span>
95-
{{! Title row: shortcut name + always-visible muted parent attribution }}
96100
<span class="snm-dropdown-card-title-group">
97101
<span class="snm-dropdown-card-title">{{item.title}}</span>
98102
{{#if item._parentTitle}}
99103
<span class="snm-dropdown-card-parent-label" aria-label="from {{item._parentTitle}}"{{item._parentTitle}}</span>
100104
{{/if}}
101105
</span>
102-
</LinkToExternal>
103-
{{#unless @atPinnedLimit}}
104-
<button
105-
type="button"
106-
class="snm-dropdown-pin-btn"
107-
title="Pin to navigation bar"
108-
aria-label="Pin {{item.title}} to navigation bar"
109-
{{on "click" (fn @onQuickPin item)}}
110-
>
111-
<FaIcon @icon="thumbtack" @size="xs" />
112-
</button>
113-
{{/unless}}
114-
</div>
115-
{{#if item.description}}
116-
<p class="snm-dropdown-card-description">{{item.description}}</p>
117-
{{else if item._parentTitle}}
118-
<p class="snm-dropdown-card-description snm-dropdown-card-description--from"><em>from {{item._parentTitle}}</em></p>
119-
{{/if}}
106+
</div>
107+
{{#if item.description}}
108+
<p class="snm-dropdown-card-description">{{item.description}}</p>
109+
{{else if item._parentTitle}}
110+
<p class="snm-dropdown-card-description snm-dropdown-card-description--from"><em>from {{item._parentTitle}}</em></p>
111+
{{/if}}
112+
</LinkToExternal>
113+
{{#unless @atPinnedLimit}}
114+
<button
115+
type="button"
116+
class="snm-dropdown-pin-btn"
117+
title="Pin to navigation bar"
118+
aria-label="Pin {{item.title}} to navigation bar"
119+
{{on "click" (fn @onQuickPin item)}}
120+
>
121+
<FaIcon @icon="thumbtack" @size="xs" />
122+
</button>
123+
{{/unless}}
120124
</div>
121125
{{else}}
122126
{{! ── Primary extension card ──────────────────────────── }}
123-
<div class="snm-dropdown-card" role="presentation">
124-
<div class="snm-dropdown-card-header">
125-
{{#if item.onClick}}
126-
<a
127-
href="javascript:;"
128-
class="snm-dropdown-card-link"
129-
title={{item.title}}
130-
{{on "click" (fn this.handleItemClick item)}}
131-
>
127+
<div class="snm-dropdown-card-wrap">
128+
{{#if item.onClick}}
129+
<a
130+
href="javascript:;"
131+
class="snm-dropdown-card"
132+
title={{item.title}}
133+
{{on "click" (fn this.handleItemClick item)}}
134+
>
135+
<div class="snm-dropdown-card-header">
132136
<span class="snm-dropdown-card-icon">
133137
{{#if item.iconComponent}}
134138
{{component (lazy-engine-component item.iconComponent) options=item.iconComponentOptions}}
@@ -142,14 +146,19 @@
142146
<span class="snm-dropdown-card-parent-label" aria-label="from {{item._parentTitle}}"{{item._parentTitle}}</span>
143147
{{/if}}
144148
</span>
145-
</a>
146-
{{else}}
147-
<LinkToExternal
148-
@route={{item.route}}
149-
id={{concat (dasherize (or item.route item.id "nav")) "-dropdown-card"}}
150-
class="snm-dropdown-card-link"
151-
title={{item.title}}
152-
>
149+
</div>
150+
{{#if item.description}}
151+
<p class="snm-dropdown-card-description">{{item.description}}</p>
152+
{{/if}}
153+
</a>
154+
{{else}}
155+
<LinkToExternal
156+
@route={{item.route}}
157+
id={{concat (dasherize (or item.route item.id "nav")) "-dropdown-card"}}
158+
class="snm-dropdown-card"
159+
title={{item.title}}
160+
>
161+
<div class="snm-dropdown-card-header">
153162
<span class="snm-dropdown-card-icon">
154163
{{#if item.iconComponent}}
155164
{{component (lazy-engine-component item.iconComponent) options=item.iconComponentOptions}}
@@ -163,23 +172,23 @@
163172
<span class="snm-dropdown-card-parent-label" aria-label="from {{item._parentTitle}}"{{item._parentTitle}}</span>
164173
{{/if}}
165174
</span>
166-
</LinkToExternal>
167-
{{/if}}
168-
{{#unless @atPinnedLimit}}
169-
<button
170-
type="button"
171-
class="snm-dropdown-pin-btn"
172-
title="Pin to navigation bar"
173-
aria-label="Pin {{item.title}} to navigation bar"
174-
{{on "click" (fn @onQuickPin item)}}
175-
>
176-
<FaIcon @icon="thumbtack" @size="xs" />
177-
</button>
178-
{{/unless}}
179-
</div>
180-
{{#if item.description}}
181-
<p class="snm-dropdown-card-description">{{item.description}}</p>
175+
</div>
176+
{{#if item.description}}
177+
<p class="snm-dropdown-card-description">{{item.description}}</p>
178+
{{/if}}
179+
</LinkToExternal>
182180
{{/if}}
181+
{{#unless @atPinnedLimit}}
182+
<button
183+
type="button"
184+
class="snm-dropdown-pin-btn"
185+
title="Pin to navigation bar"
186+
aria-label="Pin {{item.title}} to navigation bar"
187+
{{on "click" (fn @onQuickPin item)}}
188+
>
189+
<FaIcon @icon="thumbtack" @size="xs" />
190+
</button>
191+
{{/unless}}
183192
</div>
184193
{{/if}}
185194
{{/each}}

addon/components/layout/header/smart-nav-menu/dropdown.js

Lines changed: 7 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import Component from '@glimmer/component';
22
import { tracked } from '@glimmer/tracking';
33
import { action } from '@ember/object';
4-
import { dasherize } from '@ember/string';
54
import { isArray } from '@ember/array';
65
import { htmlSafe } from '@ember/template';
76

@@ -25,62 +24,16 @@ export default class LayoutHeaderSmartNavMenuDropdownComponent extends Component
2524
}
2625

2726
/**
28-
* Expand every MenuItem's shortcuts array into sibling flat items.
29-
* The resulting array interleaves parent items and their shortcuts in
30-
* registration order, matching the AWS Console pattern.
27+
* Returns the items array as-is for filtering.
3128
*
32-
* Each shortcut is normalised to:
33-
* { title, route, icon, iconPrefix, id, _isShortcut: true, _parentTitle }
29+
* Shortcuts are already registered as first-class items in the universe
30+
* registry (with `_isShortcut: true` and `_parentTitle` set) by
31+
* `menu-service.registerHeaderMenuItem()` at boot time. There is no need
32+
* to expand `item.shortcuts` here — doing so would produce a duplicate card
33+
* for every shortcut (one from the registry, one from the expansion).
3434
*/
3535
get expandedItems() {
36-
const items = this.args.items ?? [];
37-
const result = [];
38-
for (const item of items) {
39-
result.push(item);
40-
if (isArray(item.shortcuts)) {
41-
for (const sc of item.shortcuts) {
42-
const scId = sc.id ?? dasherize(item.id + '-sc-' + sc.title);
43-
result.push({
44-
// ── Identity ────────────────────────────────────────
45-
id: scId,
46-
slug: sc.slug ?? scId,
47-
title: sc.title,
48-
text: sc.text ?? sc.title,
49-
label: sc.label ?? sc.title,
50-
51-
// ── Routing ──────────────────────────────────────────
52-
route: sc.route ?? item.route,
53-
queryParams: sc.queryParams ?? {},
54-
routeParams: sc.routeParams ?? [],
55-
56-
// ── Icons (full surface) ─────────────────────────────
57-
icon: sc.icon ?? item.icon ?? 'arrow-right',
58-
iconPrefix: sc.iconPrefix ?? item.iconPrefix ?? null,
59-
iconSize: sc.iconSize ?? null,
60-
iconClass: sc.iconClass ?? null,
61-
iconComponent: sc.iconComponent ?? null,
62-
iconComponentOptions: sc.iconComponentOptions ?? {},
63-
64-
// ── Metadata ─────────────────────────────────────────
65-
description: sc.description ?? null,
66-
tags: isArray(sc.tags) ? sc.tags : isArray(item.tags) ? item.tags : null,
67-
68-
// ── Behaviour ────────────────────────────────────────
69-
onClick: sc.onClick ?? null,
70-
disabled: sc.disabled ?? false,
71-
72-
// ── Styling ───────────────────────────────────────────
73-
class: sc.class ?? null,
74-
75-
// ── Internal flags ────────────────────────────────────
76-
_isShortcut: true,
77-
_parentTitle: item.title,
78-
_parentId: item.id,
79-
});
80-
}
81-
}
82-
}
83-
return result;
36+
return this.args.items ?? [];
8437
}
8538

8639
get filteredItems() {

0 commit comments

Comments
 (0)