[SideNav] Add badges and external link icon#230787
[SideNav] Add badges and external link icon#230787weronikaolejniczak merged 10 commits intoelastic:mainfrom
Conversation
4491158 to
a26deda
Compare
|
Pinging @elastic/eui-team (EUI) |
9761768 to
589827a
Compare
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
|
|
I was puzzled why no change in the kibana mapper and no ci failure. Looks like because we use interfaces, ts doesn't complain about external -> isExternal change here: :( I can patch up in separate pr |
| label: warnIfMissing(child, 'title', 'Missing Title 😭'), | ||
| href: warnIfMissing(child, 'href', 'Missing Href 😭'), | ||
| external: child.isExternalLink, | ||
| iconType: child.icon, |
There was a problem hiding this comment.
@Dosant I'm wondering about this here. Do we already use the "Add data" pattern? Do we need a user-defined left-side icon?
There was a problem hiding this comment.
I don't see it in the current tree, but I saw it in Figma. So, maybe it's not there yet, but it could be in the future.
mgadewoll
left a comment
There was a problem hiding this comment.
🟢 Code changes LGTM from EUI side. Thanks for applying the earlier suggestions!
## Summary Resolves elastic/eui-private#395 Changes: - Implemented [Beta badge](https://www.figma.com/design/SDtdvdzPcaDZRRXMV02gSW/Solution-Side-Navigation--9.X-?node-id=6216-53843&t=v0z5JRkI5ZdoskcE-4) (and Tech Preview badge) for the cases: - expanded mode primary menu item tooltip ("Beta/Tech preview" + inverted badge), - collapsed mode primary menu item tooltip (menu label + inverted badge), - secondary menu header (badge), - secondary menu item (badge), - footer item tooltip (menu label + inverted badge), - "More" menu item children (badge). - Fixed the tooltip persistence behavior when clicking on the trigger (default EUI behavior but can obstruct popover content). - Added `popout` icon to external secondary menu items. - Changed the popover bottom gap from `4px` to `17px` to align with the last item in the footer. - Added a flex `div` to the navigation root with `data-test-subj`. - Fixed the side navigation overflow. [EuiBetaBadge component](https://eui.elastic.co/docs/components/display/badge/#beta-badge) ## Screenshots and videos ### External link <img width="806" height="622" alt="image" src="https://github.com/user-attachments/assets/cc5bfdf4-4837-41ee-8d9a-94081eb52a02" /> ### Badge | Expanded | Collapsed | | ---------- | ---------- | |  |  | ### Popover bottom gap | Before | After | | ------- | ----- | | <img width="617" height="893" alt="image" src="https://github.com/user-attachments/assets/554e20c0-af08-4d30-8ca2-5e01bb3c09d1" /> | <img width="738" height="890" alt="image" src="https://github.com/user-attachments/assets/c7add5bd-ceec-4059-8e51-92e048e6709d" /> | ## 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.
## Summary Resolves elastic/eui-private#395 Changes: - Implemented [Beta badge](https://www.figma.com/design/SDtdvdzPcaDZRRXMV02gSW/Solution-Side-Navigation--9.X-?node-id=6216-53843&t=v0z5JRkI5ZdoskcE-4) (and Tech Preview badge) for the cases: - expanded mode primary menu item tooltip ("Beta/Tech preview" + inverted badge), - collapsed mode primary menu item tooltip (menu label + inverted badge), - secondary menu header (badge), - secondary menu item (badge), - footer item tooltip (menu label + inverted badge), - "More" menu item children (badge). - Fixed the tooltip persistence behavior when clicking on the trigger (default EUI behavior but can obstruct popover content). - Added `popout` icon to external secondary menu items. - Changed the popover bottom gap from `4px` to `17px` to align with the last item in the footer. - Added a flex `div` to the navigation root with `data-test-subj`. - Fixed the side navigation overflow. [EuiBetaBadge component](https://eui.elastic.co/docs/components/display/badge/#beta-badge) ## Screenshots and videos ### External link <img width="806" height="622" alt="image" src="https://github.com/user-attachments/assets/cc5bfdf4-4837-41ee-8d9a-94081eb52a02" /> ### Badge | Expanded | Collapsed | | ---------- | ---------- | |  |  | ### Popover bottom gap | Before | After | | ------- | ----- | | <img width="617" height="893" alt="image" src="https://github.com/user-attachments/assets/554e20c0-af08-4d30-8ca2-5e01bb3c09d1" /> | <img width="738" height="890" alt="image" src="https://github.com/user-attachments/assets/c7add5bd-ceec-4059-8e51-92e048e6709d" /> | ## 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.
#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>
Summary
Resolves https://github.com/elastic/eui-private/issues/395
Changes:
popouticon to external secondary menu items.4pxto17pxto align with the last item in the footer.divto the navigation root withdata-test-subj.EuiBetaBadge component
Screenshots and videos
External link
Badge
Popover bottom gap
QA
Run:
yarn storybook shared_uxNavigate to
/?path=/story/chrome-navigation--defaultChecklist
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)release_note:*label is applied per the guidelinesbackport:*labels.