[SideNav] Make logo clickable and minor improvements by weronikaolejniczak · Pull Request #230392 · elastic/kibana

weronikaolejniczak · 2025-08-04T11:08:17Z

Summary

Resolves https://github.com/elastic/eui-private/issues/394

Changes:

The logo is now clickable as per a popular request. You can pass href in logo prop to the <Navigation> component.
Interface has changed, now logo properties are grouped under the logo prop.
isCurrent props are renamed to isActive and active management state is handled based on the item ids. So, if a parent and a child have the same id, they both become active. From the navigation perspective, they are the same navigational element.
- For correct active state management, logo requires an id as well.
All menu items are links, except for the "Menu" trigger which doesn't navigate. Therefore, all menu items support the browser context menu "Open in a new tab" as well as middle click and CMD + Click.
All menu item labels are 11px and have a weight of 500, except for the logo which has 600.
Navigation interfaces are exported from the package so they can be used externally by consumers.

QA

Run:
yarn storybook shared_ux

Navigate to /?path=/story/chrome-navigation--default

Checklist

~~Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~ (will be tackled on another issue)
~~Unit or functional tests were updated or added to match the most common scenarios~~ (will be tackled on another issue)
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
Review the backport guidelines and apply applicable backport:* labels.

elasticmachine · 2025-08-04T11:26:00Z

Pinging @elastic/eui-team (EUI)

Dosant

👍

src/core/packages/chrome/navigation/src/components/navigation.tsx

weronikaolejniczak · 2025-08-06T11:48:46Z

@Dosant I updated the implementation, could you check? I also tried to update the usage but I'm sure I'm missing some things. Feel free to commit directly to this PR!

Fyi I didn't update the snapshot tests...

export navigation props

Dosant · 2025-08-06T14:30:58Z

noticed a warning in the console:

Warning: Received `false` for a non-boolean attribute `external`.

If you want to write it to the DOM, pass a string instead: external="false" or external={value.toString()}.

If you used to conditionally omit it with external={condition && value}, pass external={condition ? value : undefined} instead. Error Component Stack
 at a (<anonymous>)
    at emotion-element-c39617d8.browser.esm.js:37:1
    at EuiButtonEmpty (button_empty.js:32:1)
    at emotion-element-c39617d8.browser.esm.js:37:1
    at li (<anonymous>)
    at SecondaryMenuItemComponent (item.tsx:20:1)

weronikaolejniczak · 2025-08-06T15:31:48Z

@Dosant I've just fixed the active state management. It uses a derived state which is an anti-pattern, I can clean this up later. It should work for now 👍🏻

Dosant · 2025-08-06T15:34:05Z

@weronikaolejniczak awesome! I see that the active state of the primary menu items looks good, but the secondary items don't seem to be picking it up. Specifically in the footer > stack management.

Dosant · 2025-08-06T15:36:35Z

@weronikaolejniczak not sure if this is a mistake on my or your side yet

weronikaolejniczak · 2025-08-06T15:37:30Z

@Dosant seems to be working fine for me in Storybook at both levels 🤔 I'm launching Kibana now, will see

Dosant · 2025-08-06T15:38:49Z

@weronikaolejniczak I realized that this seems to be a problem specifically with the footer secondary items. I'm not sure if the problem is on your side or mine.

weronikaolejniczak · 2025-08-06T15:39:58Z

@Dosant great, thanks for narrowing it down! I can confirm it's on my side, fixing it...

…no-cache --fix'

weronikaolejniczak · 2025-08-06T16:00:25Z

@Dosant works now!

Dosant · 2025-08-06T16:23:05Z

🚢

Dosant · 2025-08-06T18:21:42Z

Oh no, I'll fix...

elasticmachine · 2025-08-06T20:05:01Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`core`	525	527	+2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/core-chrome-navigation`	4	30	+26

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`core`	108.9KB	110.5KB	+1.6KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/core`	892	896	+4

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	441.0KB	441.0KB	-1.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/core-chrome-navigation`	4	39	+35

History

cc @weronikaolejniczak

mgadewoll

I added a couple small suggestions to connect some styles with theme tokens. Besides that from a quick code check only, the updates are LGTM.

mgadewoll · 2025-08-07T09:25:17Z

src/core/packages/chrome/navigation/src/components/menu_item/index.tsx

+    const horizontalStyles =
+      !isHorizontal &&
+      css`
+        font-size: 11px;


Suggestion: We could use a proper theme font size here: euiFontSize(euiThemeContext, 'xxs', { unit: 'px' }).fontSize

src/core/packages/chrome/navigation/src/components/side_nav/logo.tsx

src/core/packages/chrome/navigation/src/components/menu_item/index.tsx

weronikaolejniczak · 2025-08-07T09:38:01Z

Thanks for the suggestions @mgadewoll! I'll add them on another PR: #230787

## Summary Resolves elastic/eui-private#394 Changes: - The logo is now clickable as per a popular request. You can pass `href` in `logo` prop to the `<Navigation>` component. - Interface has changed, now logo properties are grouped under the `logo` prop. - `isCurrent` props are renamed to `isActive` and active management state is handled based on the item `id`s. So, if a parent and a child have the same `id`, they both become active. From the navigation perspective, they are the same navigational element. - For correct active state management, logo requires an `id` as well.\ - All menu items are links, except for the "Menu" trigger which doesn't navigate. Therefore, all menu items support the browser context menu "Open in a new tab" as well as middle click and <kbd>CMD</kbd> + Click. - All menu item labels are `11px` and have a weight of `500`, except for the logo which has `600`. - Navigation interfaces are exported from the package so they can be used externally by consumers. ![Kapture 2025-08-06 at 13 32 14](https://github.com/user-attachments/assets/b7875c79-df99-4184-ac17-5c4602d3bf3a) <img width="820" height="1028" alt="Screenshot 2025-08-06 at 12 52 38" src="https://github.com/user-attachments/assets/c12543f4-e608-4e1d-a7df-7d911ec61fa1" /> <img width="832" height="1026" alt="Screenshot 2025-08-06 at 12 53 03" src="https://github.com/user-attachments/assets/dee356fe-84c9-4459-a9cf-c997504b9cdb" /> <img width="826" height="973" alt="Screenshot 2025-08-06 at 13 18 51" src="https://github.com/user-attachments/assets/937179cf-dc6f-4b3e-b331-37606362d4f7" /> <img width="821" height="973" alt="Screenshot 2025-08-06 at 13 19 04" src="https://github.com/user-attachments/assets/4ac58bb8-3ce3-420c-b032-5500966e34b8" /> ## QA Run: `yarn storybook shared_ux` Navigate to `/?path=/story/chrome-navigation--default` ### Checklist - [ ] ~Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)~ (will be tackled on [another issue](elastic/eui-private#371)) - [ ] ~[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~ (will be tackled on [another issue](elastic/eui-private#377)) - [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. --------- Co-authored-by: Anton Dosov <anton.dosov@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

#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: - #228162 - #230392 - #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 <42973632+kibanamachine@users.noreply.github.com>

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 <42973632+kibanamachine@users.noreply.github.com>

#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: - #228162 - #230392 - #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 <42973632+kibanamachine@users.noreply.github.com>

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 <42973632+kibanamachine@users.noreply.github.com>

weronikaolejniczak self-assigned this Aug 4, 2025

weronikaolejniczak changed the title ~~Feat/side nav logo clickable~~ feat(navigation): make logo clickable Aug 4, 2025

weronikaolejniczak requested a review from Dosant August 4, 2025 11:25

weronikaolejniczak marked this pull request as ready for review August 4, 2025 11:25

weronikaolejniczak requested review from a team as code owners August 4, 2025 11:25

weronikaolejniczak force-pushed the feat/side-nav-logo-clickable branch from d2512d7 to 22f1584 Compare August 4, 2025 11:26

Dosant approved these changes Aug 4, 2025

View reviewed changes

Dosant self-requested a review August 6, 2025 08:49

Dosant reviewed Aug 6, 2025

View reviewed changes

src/core/packages/chrome/navigation/src/components/navigation.tsx Outdated Show resolved Hide resolved

weronikaolejniczak added 9 commits August 6, 2025 13:38

feat(navigation): make logo clickable

ad26d69

fix(navigation): make side nav logo label textParagraph

e182493

feat(navigation): reuse menu item for logo

542217b

chore(navigation): prevent link navigation in Storybook

3acaf2f

chore(navigation): export navigation interfaces

2958231

refactor(navigation): use id for comparison instead of href

a626754

feat(navigation): determine the initial menu item

33afbfa

feat(navigation): handle logo active state

c30946b

feat: update navigation usage

795d603

weronikaolejniczak force-pushed the feat/side-nav-logo-clickable branch from f936169 to 795d603 Compare August 6, 2025 11:47

Dosant added 2 commits August 6, 2025 16:01

export NavigationProps

2e20844

export navigation props

adjust home node integration

fc0310b

fix(navigation): make the internal active states update to activeItemId

93facd7

weronikaolejniczak requested a review from Dosant August 6, 2025 15:33

kibanamachine and others added 2 commits August 6, 2025 15:49

[CI] Auto-commit changed files from 'node scripts/eslint_all_files --…

0366217

…no-cache --fix'

fix(navigation): handle active state of the footer secondary menu items

780086c

Dosant approved these changes Aug 6, 2025

View reviewed changes

fix test

dcba4db

mgadewoll approved these changes Aug 7, 2025

View reviewed changes

weronikaolejniczak merged commit bdc025b into elastic:main Aug 7, 2025
13 checks passed

kibanamachine added the v9.2.0 label Aug 7, 2025

weronikaolejniczak mentioned this pull request Aug 26, 2025

[SideNav] Unit test coverage, refactor active state management #232925

Merged

7 tasks

weronikaolejniczak changed the title ~~feat(navigation): make logo clickable~~ [SideNav] Make logo clickable and minor improvements Sep 23, 2025

weronikaolejniczak deleted the feat/side-nav-logo-clickable branch February 10, 2026 10:04

Conversation

weronikaolejniczak commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

QA

Checklist

Uh oh!

elasticmachine commented Aug 4, 2025

Uh oh!

Dosant left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weronikaolejniczak commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dosant commented Aug 6, 2025

Uh oh!

weronikaolejniczak commented Aug 6, 2025

Uh oh!

Dosant commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dosant commented Aug 6, 2025

Uh oh!

weronikaolejniczak commented Aug 6, 2025

Uh oh!

Dosant commented Aug 6, 2025

Uh oh!

weronikaolejniczak commented Aug 6, 2025

Uh oh!

weronikaolejniczak commented Aug 6, 2025

Uh oh!

Dosant commented Aug 6, 2025

Uh oh!

Dosant commented Aug 6, 2025

Uh oh!

elasticmachine commented Aug 6, 2025

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

Uh oh!

mgadewoll left a comment

Choose a reason for hiding this comment

Uh oh!

mgadewoll Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

weronikaolejniczak commented Aug 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

weronikaolejniczak commented Aug 4, 2025 •

edited

Loading

weronikaolejniczak commented Aug 6, 2025 •

edited

Loading

Dosant commented Aug 6, 2025 •

edited

Loading