Skip to content

Conversation

@weronikaolejniczak
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak commented Jul 16, 2025

Summary

This PR introduces the side navigation components into Kibana Shared UX Storybook and composes with the new grid layout in Kibana.

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

Instructions

Storybook

To test, run:

yarn storybook shared_ux
Expanded Expanded (side panel) Collapsed
image image image

Kibana

To test, run Kibana with:

feature_flags.overrides:
  core.chrome.layoutType: 'grid'
  core.chrome.projectSideNav: 'v2'

and make sure to create a space with non-classic navigation.

Screenshot 2025-07-18 at 15 04 16

Specification

WIP

Acceptance criteria

WIP

Unit tests

WIP

Checklist

  • Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support (it will be done on another PR)
  • Unit or functional tests were updated or added to match the most common scenarios (it will be done on another PR)
  • 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.

@weronikaolejniczak weronikaolejniczak changed the title feat: add side navigation components Side navigation implementation - Storybook Jul 16, 2025
@weronikaolejniczak weronikaolejniczak self-assigned this Jul 16, 2025
@weronikaolejniczak weronikaolejniczak changed the title Side navigation implementation - Storybook 🚧 Side navigation implementation - Storybook Jul 16, 2025
@weronikaolejniczak weronikaolejniczak requested review from a team July 16, 2025 10:28
@weronikaolejniczak weronikaolejniczak added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Jul 16, 2025
@weronikaolejniczak weronikaolejniczak force-pushed the feat/global-navigation-components branch 6 times, most recently from fff1ec3 to 23349ef Compare July 18, 2025 12:43
@weronikaolejniczak weronikaolejniczak changed the title 🚧 Side navigation implementation - Storybook 🚧 Side navigation implementation Jul 21, 2025
@kibanamachine
Copy link
Contributor

Cloud deployments require a Github label, please add ci:cloud-deploy or ci:cloud-redeploy and trigger the job through the checkbox again.

@weronikaolejniczak weronikaolejniczak force-pushed the feat/global-navigation-components branch 3 times, most recently from d564d8f to 0004ce6 Compare July 24, 2025 13:57
@weronikaolejniczak weronikaolejniczak force-pushed the feat/global-navigation-components branch from bf8d45d to 7be1bc4 Compare July 30, 2025 11:12
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
core 484 525 +41

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 - 5 +5

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 890 892 +2

Page load bundle

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

id before after diff
core 481.5KB 507.3KB +25.7KB
Unknown metric groups

API count

id before after diff
@kbn/core-chrome-navigation - 5 +5

ESLint disabled line counts

id before after diff
@kbn/core-chrome-navigation - 2 +2

Total ESLint disabled count

id before after diff
@kbn/core-chrome-navigation - 2 +2

History

cc @Dosant @weronikaolejniczak

@weronikaolejniczak weronikaolejniczak changed the title 🚧 Side navigation implementation Side navigation implementation Jul 31, 2025
@weronikaolejniczak weronikaolejniczak enabled auto-merge (squash) July 31, 2025 14:20
@weronikaolejniczak weronikaolejniczak merged commit eb5acc9 into elastic:main Jul 31, 2025
14 checks passed
delanni pushed a commit to delanni/kibana that referenced this pull request Aug 5, 2025
## Summary

This PR introduces the side navigation components into Kibana Shared UX
Storybook and composes with the new grid layout in Kibana.

Resolves elastic/eui-private#369
Resolves elastic/eui-private#378

### Instructions

#### Storybook

To test, run:

```bash
yarn storybook shared_ux
```

| Expanded | Expanded (side panel) | Collapsed |
| ---------- | ---------------------- | ---------- |
|
![image](https://github.com/user-attachments/assets/bccf81ad-b2a6-4ce0-904c-c6e637955f9d)
|
![image](https://github.com/user-attachments/assets/41b22ede-5a99-4f82-9df5-3a0583caaa6c)
|
![image](https://github.com/user-attachments/assets/71aea460-17b9-4fc0-919e-ac8196dacbe3)
|

#### Kibana

To test, run Kibana with:

```
feature_flags.overrides:
  core.chrome.layoutType: 'grid'
  core.chrome.projectSideNav: 'v2'
```

and make sure to create a space with non-classic navigation.

<img width="1691" height="1063" alt="Screenshot 2025-07-18 at 15 04 16"
src="https://github.com/user-attachments/assets/17fb4db9-68fe-4443-b8ee-8c191f320393"
/>

### Specification

WIP

#### Acceptance criteria

WIP

### Unit tests

WIP

### 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)~
(it will be done on another PR)
- [ ] ~[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~ (it will be
done on another PR)
- [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>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@wildemat wildemat mentioned this pull request Aug 7, 2025
10 tasks
NicholasPeretti pushed a commit to NicholasPeretti/kibana that referenced this pull request Aug 18, 2025
## Summary

This PR introduces the side navigation components into Kibana Shared UX
Storybook and composes with the new grid layout in Kibana.

Resolves elastic/eui-private#369
Resolves elastic/eui-private#378

### Instructions

#### Storybook

To test, run:

```bash
yarn storybook shared_ux
```

| Expanded | Expanded (side panel) | Collapsed |
| ---------- | ---------------------- | ---------- |
|
![image](https://github.com/user-attachments/assets/bccf81ad-b2a6-4ce0-904c-c6e637955f9d)
|
![image](https://github.com/user-attachments/assets/41b22ede-5a99-4f82-9df5-3a0583caaa6c)
|
![image](https://github.com/user-attachments/assets/71aea460-17b9-4fc0-919e-ac8196dacbe3)
|

#### Kibana

To test, run Kibana with:

```
feature_flags.overrides:
  core.chrome.layoutType: 'grid'
  core.chrome.projectSideNav: 'v2'
```

and make sure to create a space with non-classic navigation.

<img width="1691" height="1063" alt="Screenshot 2025-07-18 at 15 04 16"
src="https://github.com/user-attachments/assets/17fb4db9-68fe-4443-b8ee-8c191f320393"
/>

### Specification

WIP

#### Acceptance criteria

WIP

### Unit tests

WIP

### 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)~
(it will be done on another PR)
- [ ] ~[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~ (it will be
done on another PR)
- [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>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
weronikaolejniczak added a commit that referenced this pull request Sep 16, 2025
#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>
@weronikaolejniczak weronikaolejniczak changed the title Side navigation implementation [SideNav] Move components from demo env to Storybook Sep 23, 2025
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Sep 24, 2025
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>
niros1 pushed a commit that referenced this pull request Sep 30, 2025
#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>
rylnd pushed a commit to rylnd/kibana that referenced this pull request Oct 17, 2025
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting EUI release_note:skip Skip the PR/issue when compiling release notes v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants