Skip to content

Commit acf03a0

Browse files
weronikaolejniczakkibanamachine
authored andcommitted
[Side navigation] Unit test coverage, refactor active state management (elastic#232925)
## Summary Resolves elastic/eui-private#377 On this PR, I am adding unit tests that cover the whole side navigation functionality. See previous PRs: - elastic#228162 - elastic#230392 - elastic#230787 ### Changes - Added test suites: `both_modes.test.tsx` (expanded + collapsed mode), `collapsed_mode.test.tsx`, `expanded_mode.test.tsx`. - Refactored the active state management to be fully controlled by `activeItemId`, no internal state. - Refactored `isActive` to be: `isCurrent` (`aria-current="page"`) and `isHighlighted` (visual indication). - Added `onItemClick` to the navigation component interface. - Changed the primary menu limit to 12. - Improved the roving index. ### Motivation The reason I'm adding the tests afterwards is because we were iterating fast, the requirements changed frequently and the acceptance criteria were not documented anywhere. With this testing suite, I was able to refactor the active management state to be fully controlled through `activeItemId` prop without introducing regression. ### Coverage File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ------------------------------------|---------|----------|---------|---------|------------------- All files | 78.17 | 71.47 | 70.77 | 79.48 | core/packages/chrome/navigation | 0 | 0 | 0 | 0 | types.ts | 0 | 0 | 0 | 0 | .../packages/chrome/navigation/src | 100 | 100 | 100 | 100 | constants.ts | 100 | 100 | 100 | 100 | ...rome/navigation/src/__stories__ | 0 | 0 | 0 | 0 | navigation.stories.tsx | 0 | 0 | 0 | 0 | 23-200 ...chrome/navigation/src/__tests__ | 100 | 100 | 100 | 100 | resize_window.ts | 100 | 100 | 100 | 100 | ...hrome/navigation/src/components | 84.9 | 82.85 | 92.85 | 84.9 | navigation.tsx | 84.9 | 82.85 | 92.85 | 84.9 | ...44-146,319-321 ...omponents/nested_secondary_menu | 94.87 | 69.23 | 100 | 94.87 | header.tsx | 100 | 100 | 100 | 100 | menu_item.tsx | 90 | 50 | 100 | 90 | 60 menu_panel.tsx | 100 | 100 | 100 | 100 | primary_menu_item.tsx | 100 | 75 | 100 | 100 | 31-32 use_nested_menu.ts | 83.33 | 50 | 100 | 83.33 | 24 ...vigation/src/components/popover | 95 | 90.32 | 88.23 | 100 | blur_popover.ts | 100 | 100 | 100 | 100 | use_keyboard_management.ts | 96.15 | 80 | 100 | 100 | 37,48-54 use_persistent_popover.ts | 100 | 100 | 100 | 100 | use_popover_hover.ts | 100 | 100 | 100 | 100 | use_popover_open.ts | 80 | 100 | 60 | 100 | ...n/src/components/secondary_menu | 100 | 80 | 100 | 100 | item.tsx | 100 | 70.58 | 100 | 100 | 42-47,61,86-100 section.tsx | 100 | 100 | 100 | 100 | ...igation/src/components/side_nav | 100 | 94 | 100 | 100 | footer.tsx | 100 | 100 | 100 | 100 | footer_item.tsx | 100 | 92.85 | 100 | 100 | 55 logo.tsx | 100 | 100 | 100 | 100 | panel.tsx | 100 | 100 | 100 | 100 | primary_menu.tsx | 100 | 100 | 100 | 100 | primary_menu_item.tsx | 100 | 92.3 | 100 | 100 | 82-92 ...ges/chrome/navigation/src/hooks | 96.49 | 76.74 | 96 | 98.14 | use_hover_timeout.ts | 100 | 100 | 100 | 100 | use_layout_width.ts | 100 | 83.33 | 100 | 100 | 27 use_menu_header_style.ts | 100 | 50 | 100 | 100 | 25-32 use_navigation.ts | 100 | 100 | 100 | 100 | use_responsive_menu.ts | 94.73 | 68.75 | 80 | 94.44 | 47,63 use_roving_index.ts | 94.73 | 81.81 | 100 | 100 | 25,56 use_tooltip.ts | 100 | 100 | 100 | 100 | ...ges/chrome/navigation/src/utils | 87.35 | 75 | 93.75 | 92 | calculate_item_visible_count.ts | 100 | 83.33 | 100 | 100 | 28 focus_first_element.ts | 85.71 | 50 | 100 | 100 | 19-23 focus_main_content.ts | 75 | 50 | 100 | 75 | 16 get_actual_item_heights.ts | 100 | 100 | 100 | 100 | get_focusable_elements.ts | 100 | 100 | 100 | 100 | get_has_submenu.ts | 100 | 100 | 100 | 100 | get_initial_active_items.ts | 83.33 | 83.33 | 80 | 83.33 | 66-71 partition_menu_items.ts | 100 | 100 | 100 | 100 | trap_focus.ts | 75 | 62.5 | 100 | 91.66 | 29 ...kages/shared/kbn-i18n-react/src | 58.33 | 25 | 33.33 | 58.33 | compatiblity_layer.tsx | 33.33 | 0 | 0 | 33.33 | 29-32,52 inject.tsx | 0 | 0 | 0 | 0 | provider.tsx | 83.33 | 50 | 100 | 83.33 | 23 ...rm/packages/shared/kbn-i18n/src | 7.69 | 0 | 0 | 8.33 | loader.ts | 7.69 | 0 | 0 | 8.33 | 36-164 ...ckages/shared/kbn-i18n/src/core | 51.16 | 41.66 | 54.54 | 51.16 | error_handler.ts | 33.33 | 0 | 0 | 33.33 | 24-26 formats.ts | 100 | 100 | 100 | 100 | i18n.ts | 51.28 | 45.45 | 60 | 51.28 | ...83,193,207-221 .../shared/kbn-test/src/jest/setup | 88.88 | 88.88 | 100 | 88.88 | emotion.js | 88.88 | 88.88 | 100 | 88.88 | 43 **Test Suites:** 3 passed, 3 total **Tests:** 73 passed, 73 total **Snapshots:** 3 passed, 3 total **Time:** 14.984 s, estimated 16 s ## Checklist Run `node scripts/jest src/core/packages/chrome/navigation/src/ --coverage`. ### General - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Specific - [ ] All added tests pass in CI. See below 👇🏻 - [ ] All added tests should pass locally. Run `yarn test:jest src/core/packages/chrome/navigation` to check. - [ ] There should be a high coverage of unit tests in the `navigation` package. Run `yarn test:jest src/core/packages/chrome/navigation --coverage` to check. - [ ] All side navigation acceptance criteria are covered. Verify with the Google Doc notebook, "Acceptance criteria" tab. --------- Co-authored-by: kibanamachine <[email protected]>
1 parent c29edf9 commit acf03a0

35 files changed

+4392
-401
lines changed

src/core/packages/chrome/navigation/README.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,6 @@ An adaptive side navigation system built with [Elastic UI](https://eui.elastic.c
66
| ----------------------------- | ------------------------------ |
77
| ![image](./expanded-mode.png) | ![image](./collapsed-mode.png) |
88

9-
## Features
10-
11-
- **Responsive design** - automatically adapts between collapsed and expanded states based on screen size.
12-
- **Smart menu system** - dynamic "More" menu that consolidates overflow items during window resize.
13-
- **Nested navigation** - multi-level menu support with nested popover panels in collapsed mode.
14-
- **Accessibility-first** - WCAG-compliant with proper ARIA labels, keyboard navigation, and screen reader support.
15-
- **Modular architecture** - composable components with clean separation of concerns. Exported as a self-contained widget.
16-
- **Dark mode** and **High contrast mode support**
17-
189
## Usage
1910

2011
### Basic setup
@@ -66,6 +57,12 @@ const navigationItems = {
6657

6758
function App() {
6859
const [isCollapsed, setIsCollapsed] = useState(false);
60+
const [activeItemId, setActiveItemId] = useState('dashboard');
61+
62+
const handleItemClick = (item: MenuItem | SecondaryMenuItem | SideNavLogo) => {
63+
setActiveItemId(item.id);
64+
trackAnalytics(item.id);
65+
};
6966

7067
return (
7168
<div className="app">
@@ -80,6 +77,7 @@ function App() {
8077
iconType: 'observabilityApp',
8178
href: '/observability',
8279
}}
80+
onItemClick={handleItemClick}
8381
setWidth={setNavigationWidth}
8482
/>
8583
<main className="app-content">{/* Your application content */}</main>
@@ -101,6 +99,7 @@ export const navigationItems = {
10199
label: 'Overview',
102100
iconType: 'info',
103101
href: '/overview',
102+
badgeType: 'techPreview', // for tech preview items
104103
},
105104
// Menu item with nested sections
106105
{
@@ -111,7 +110,7 @@ export const navigationItems = {
111110
sections: [
112111
{
113112
id: 'reports-section',
114-
label: 'Reports', // or null for unlabeled sections
113+
label: 'Reports', // omit for unlabeled sections
115114
items: [
116115
{
117116
id: 'analytics', // has the same `id` as the parent item
@@ -122,6 +121,7 @@ export const navigationItems = {
122121
id: 'sales-report',
123122
label: 'Sales report',
124123
href: '/analytics/sales',
124+
badgeType: 'beta', // for beta items
125125
},
126126
{
127127
id: 'traffic-report',

src/core/packages/chrome/navigation/src/__stories__/navigation.stories.tsx

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export default {
7373
id: 'observability',
7474
href: LOGO.href,
7575
label: LOGO.label,
76-
iconType: LOGO.type,
76+
iconType: LOGO.iconType,
7777
},
7878
setWidth: () => {},
7979
},
@@ -91,6 +91,7 @@ export const Default: StoryObj<PropsAndArgs> = {
9191
);
9292
},
9393
],
94+
render: (args) => <ControlledNavigation {...args} />,
9495
};
9596

9697
export const Collapsed: StoryObj<PropsAndArgs> = {
@@ -108,6 +109,7 @@ export const Collapsed: StoryObj<PropsAndArgs> = {
108109
args: {
109110
isCollapsed: true,
110111
},
112+
render: (args) => <ControlledNavigation {...args} />,
111113
};
112114

113115
export const WithMinimalItems: StoryObj<PropsAndArgs> = {
@@ -128,6 +130,7 @@ export const WithMinimalItems: StoryObj<PropsAndArgs> = {
128130
footerItems: PRIMARY_MENU_FOOTER_ITEMS.slice(0, 2),
129131
},
130132
},
133+
render: (args) => <ControlledNavigation {...args} />,
131134
};
132135

133136
export const WithManyItems: StoryObj<PropsAndArgs> = {
@@ -168,6 +171,24 @@ export const WithManyItems: StoryObj<PropsAndArgs> = {
168171
footerItems: PRIMARY_MENU_FOOTER_ITEMS,
169172
},
170173
},
174+
render: (args) => <ControlledNavigation {...args} />,
175+
};
176+
177+
export const WithinLayout: StoryObj<PropsAndArgs> = {
178+
name: 'Navigation within Layout',
179+
render: (args) => <Layout {...args} />,
180+
};
181+
182+
const ControlledNavigation = ({ ...props }: PropsAndArgs) => {
183+
const [activeItemId, setActiveItemId] = useState(props.activeItemId || PRIMARY_MENU_ITEMS[0].id);
184+
185+
return (
186+
<Navigation
187+
{...props}
188+
activeItemId={activeItemId}
189+
onItemClick={(item) => setActiveItemId(item.id)}
190+
/>
191+
);
171192
};
172193

173194
const Layout = ({ ...props }: PropsAndArgs) => {
@@ -212,15 +233,7 @@ const Layout = ({ ...props }: PropsAndArgs) => {
212233
backgroundColor={euiTheme.colors.backgroundFilledText}
213234
/>
214235
}
215-
navigation={
216-
<Navigation
217-
activeItemId={props.activeItemId}
218-
isCollapsed={props.isCollapsed}
219-
items={props.items}
220-
logo={props.logo}
221-
setWidth={setNavigationWidth}
222-
/>
223-
}
236+
navigation={<ControlledNavigation {...props} setWidth={setNavigationWidth} />}
224237
sidebar={
225238
<Box
226239
label="Global Sidebar"
@@ -260,8 +273,3 @@ const Layout = ({ ...props }: PropsAndArgs) => {
260273
</>
261274
);
262275
};
263-
264-
export const WithinLayout: StoryObj<PropsAndArgs> = {
265-
name: 'Navigation within Layout',
266-
render: (args) => <Layout {...args} />,
267-
};

0 commit comments

Comments
 (0)