feat: add per-group expandable control to button dropdown - #4947
feat: add per-group expandable control to button dropdown#4947revezard wants to merge 9 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4947 +/- ##
==========================================
- Coverage 97.66% 93.76% -3.91%
==========================================
Files 959 958 -1
Lines 31342 27144 -4198
Branches 11577 9763 -1814
==========================================
- Hits 30610 25451 -5159
+ Misses 725 613 -112
- Partials 7 1080 +1073 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| import styles from './styles.scss'; | ||
|
|
||
| // Prototype for per-group expandability (inherit-override): `expandableGroups` is the |
There was a problem hiding this comment.
Cleaned up comments
| ]; | ||
|
|
||
| export default function MixedExpandableScenario() { | ||
| const [expandToViewport, setExpandToViewport] = useState(false); |
There was a problem hiding this comment.
It would be better to store this configuration in the URL, same as here, so the page is easier to share
| // the window, using the built-in override symbol read by the useMobile hook. Dispatching a | ||
| // resize event makes the useMobile singleton re-evaluate immediately. | ||
| useEffect(() => { | ||
| const forceMobileModeSymbol = Symbol.for('awsui-force-mobile-mode'); |
There was a problem hiding this comment.
I’d suggest removing this part, since it’s easy to set the screen size in the developer tools instead
| // it's possible to navigate nested groups. | ||
| if (next.parent?.disabled && hasExpandableGroups) { | ||
| // don't step into a disabled expandable group; a flat group's children stay navigable | ||
| if (next.parent && isExpandable(next.parent) && next.parent.disabled) { |
There was a problem hiding this comment.
Why not just next.parent?.disabled?
There was a problem hiding this comment.
This mirrors the old next.parent?.disabled && hasExpandableGroups check, just per-group: a disabled expandable group renders no submenu so its children aren't in the DOM and must be skipped, whereas a flat group's children render inline (disabled but still highlightable), so next.parent?.disabled alone would wrongly skip them.
Updated comment to be clearer.
| export const isItemGroup = (item: ButtonDropdownProps.ItemOrGroup): item is ButtonDropdownProps.ItemGroup => | ||
| item && (item as ButtonDropdownProps.ItemGroup).items !== undefined; | ||
|
|
||
| // A group's own `expandable` flag decides whether it renders as an expandable submenu, falling |
There was a problem hiding this comment.
Cleaned up comments
4 participants
Description
Adds an
expandableflag to ButtonDropdown'sItemGroupthat overrides the dropdown-levelexpandableGroupson a per-group basis:expandable: true— the group renders as an expandable submenu even whenexpandableGroupsis unsetexpandable: false— the group renders inline (flat) even whenexpandableGroupsis setexpandableGroupsvalueThis lets a single dropdown mix expandable and flat groups (e.g. keep high-traffic groups inline
while long/secondary groups collapse into submenus). Expandability is resolved per node via a
small
planeOfplane model rather than a single global flag, so expandable and flat groupscoexist correctly for keyboard navigation on both desktop and in the mobile restricted view.
The change is backward-compatible: with no
expandableflag set, behavior is identical to today.Implemented per design alignment with the Cloudscape team (<name / discussion or issue link>).
Related links, issue #, if available: <link, or n/a>
How has this been tested?
is-group-expandable.test.ts(locks theinherit/override resolution) and new mixed expandable/flat navigation cases in
move-highlight.test.tscovering both desktop plane confinement and the mobile restricted-viewpath (flat-group children reachable, collapsed expandable-group children gated).
button-dropdown/expandableand anew
button-dropdown/expandable-mixeddev page, in both light and dark modes.flat groups and into/out of expandable groups; filtering flattens all groups; toggling
expandableGroupswith per-groupexpandableoverrides behaves as documented.Reviewers can exercise it on the
button-dropdown/expandable-mixeddev page: toggleexpandableGroupsand observe the forced-expandable (true) and forced-flat (false) groups holdtheir behavior while the rest inherit.
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
expandableprop documented via JSDoc onItemGroup)expandableGroups; absent flag preserves current behavior)Security
Testing
is-group-expandable.test.ts,move-highlight.test.tsmixed-mode cases)By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.