Skip to content

[SideNav] v1 Component Clean-Up#241114

Merged
Dosant merged 13 commits intoelastic:mainfrom
Dosant:d/2025-10-29-sidenav-clean-up
Nov 3, 2025
Merged

[SideNav] v1 Component Clean-Up#241114
Dosant merged 13 commits intoelastic:mainfrom
Dosant:d/2025-10-29-sidenav-clean-up

Conversation

@Dosant
Copy link
Copy Markdown
Contributor

@Dosant Dosant commented Oct 29, 2025

Summary

  • First pass - just component and the related code in chrome

Next steps:

  • Nav tree structure
  • Functional tests helpers

@Dosant Dosant changed the title wip side nav clean up [SideNav] v1 Component Clean-Up Oct 29, 2025
@Dosant Dosant marked this pull request as ready for review October 30, 2025 10:39
@Dosant Dosant requested review from a team as code owners October 30, 2025 10:39
@Dosant Dosant added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// labels Oct 30, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

Copy link
Copy Markdown
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some README files can still be updated:

diff --git src/core/packages/chrome/browser-internal/src/project_navigation/README.md src/core/packages/chrome/browser-internal/src/project_navigation/README.md
index a400f8055a6..5ae983f706a 100644
--- src/core/packages/chrome/browser-internal/src/project_navigation/README.md
+++ src/core/packages/chrome/browser-internal/src/project_navigation/README.md
@@ -34,20 +34,8 @@ The following diagram illustrates how navigation state flows through the system
                                         |                 |
                                         +--------+--------+
                                                  |
-                                                 | NavigationKibanaProvider
-                                                 | adapts streams
-                                                 v
-                                        +------------------+
-                                        | NavigationProvider|
-                                        |                  |
-                                        |  +-----------+   |
-                                        |  | Context   |   |
-                                        |  +-----------+   |
-                                        |                  |
-                                        +--------+--------+
-                                                 |
-                                                 | useNavigation() hook
-                                                 | consumes context
+                                                 | Props passed directly
+                                                 | via useObservable()
                                                  v
                                         +------------------+
                                         | Navigation       |
@@ -56,8 +44,8 @@ The following diagram illustrates how navigation state flows through the system
                                         | - Renders UI     |
                                         | - Highlights     |
                                         |   active items   |
-                                        | - Shows          |
-                                        |   breadcrumbs    |
+                                        | - Manages panel  |
+                                        |   state          |
                                         +------------------+
 ```
 
@@ -78,8 +66,8 @@ The service does not contain any UI. Instead, it processes navigation trees regi
 
 This is the most common function of the service. It determines the currently active link and broadcasts it to the UI.
 
--   **State Stream**: The active state is exposed as an RxJS observable, `activeNodes$`, which streams the active link and its parent breadcrumbs.
--   **UI Consumption**: The `ChromeService` subscribes to this stream using the `useObservable` hook and passes the static result to the React UI as a prop.
+-   **State Stream**: The active state is exposed as an RxJS observable, `activeNodes$`, which streams the active link and its parent nodes.
+-   **UI Consumption**: The `ChromeService` subscribes to this stream using the `useObservable` hook and passes the result directly to the Navigation component as props.
 -   **Detection Algorithm**: The active node is found by the `findActiveNodes` utility, which uses two methods:
     1.  **Custom Logic (`getIsActive`)**: A node can define its own function for complex activation logic.
     2.  **Longest URL Match**: As a fallback, the utility finds the most specific link by matching the longest URL prefix.
@@ -95,9 +83,9 @@ Breadcrumbs are automatically generated from the navigation tree structure:
 
 #### Active Item Highlighting
 
-UI components use the `activeNodes$` observable to highlight the currently active navigation item:
+The Navigation component uses the active nodes information to highlight the currently active navigation item:
 
-1. The `SideNavComponent` subscribes to `activeNodes$` to determine which items to highlight
+1. The Navigation component receives `activeNodes$` data via props
 2. When rendering the navigation tree, each item compares its ID with the active node's ID
 3. If they match, the item is rendered with an "active" state (different styling)
 4. Parent nodes of the active item may also receive special styling to show the active path
diff --git src/core/packages/chrome/layout/core-chrome-layout-feature-flags/README.md src/core/packages/chrome/layout/core-chrome-layout-feature-flags/README.md
index 70eb206b5a1..3530e9663dd 100644
--- src/core/packages/chrome/layout/core-chrome-layout-feature-flags/README.md
+++ src/core/packages/chrome/layout/core-chrome-layout-feature-flags/README.md
@@ -8,19 +8,16 @@ Feature flag utilities for Kibana's Chrome layout system.
 import {
   getLayoutVersion,
   getLayoutDebugFlag,
-  getSideNavVersion,
 } from '@kbn/core-chrome-layout-feature-flags';
 
 const layoutType = getLayoutVersion(featureFlags); // 'legacy-fixed' | 'grid'
 const isDebug = getLayoutDebugFlag(featureFlags); // boolean
-const sideNavVersion = getSideNavVersion(featureFlags); // 'v1' | 'v2' | 'both'
 ```
 
 ## Feature Flags
 
 - **`LAYOUT_FEATURE_FLAG_KEY`**: Layout type selection
 - **`LAYOUT_DEBUG_FEATURE_FLAG_KEY`**: Debug mode toggle
-- **`LAYOUT_PROJECT_SIDENAV_FEATURE_FLAG_KEY`**: Side navigation version
 
 ## Functions

And, src/platform/test/functional/page_objects/solution_navigation.ts can probably be substantially updated.

@Dosant
Copy link
Copy Markdown
Contributor Author

Dosant commented Oct 30, 2025

thanks @tsullivan,

I updated the readmes!

This is the first pass, I will follow with the NavigationTree types clean up and functional tests utils clean up. These changes will touch solutions code and I didn't want to trigger them in this pr

@Dosant Dosant requested a review from tsullivan October 30, 2025 18:08
Copy link
Copy Markdown
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for updating the read me files :)

@Dosant Dosant enabled auto-merge (squash) November 3, 2025 11:24
@Dosant Dosant disabled auto-merge November 3, 2025 12:33
@Dosant Dosant enabled auto-merge (squash) November 3, 2025 12:36
@Dosant Dosant merged commit a3d2f5d into elastic:main Nov 3, 2025
14 checks passed
@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #103 / Ingest pipelines app Ingest Pipelines Pipelines list Pipelines tree Clicking on tree nodes open details panel for selected pipeline
  • [job] [logs] FTR Configs #75 / Observability Rules Rules Endpoints Metric threshold rule > alert and action creation "after all" hook for "should set correct action parameter: ruleType"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
core 557 528 -29
navigation 47 14 -33
total -62

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-browser 101 100 -1
@kbn/core-chrome-layout-feature-flags 11 7 -4
@kbn/shared-ux-chrome-navigation 40 - -40
total -45

Async chunks

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

id before after diff
core 130.7KB 107.7KB -23.0KB

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 913 909 -4
@kbn/shared-ux-chrome-navigation 1 - -1
total -5

Page load bundle

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

id before after diff
core 511.0KB 508.2KB -2.8KB
navigation 10.7KB 9.7KB -1.0KB
total -3.8KB
Unknown metric groups

API count

id before after diff
@kbn/core-chrome-browser 208 207 -1
@kbn/core-chrome-layout-feature-flags 11 7 -4
@kbn/shared-ux-chrome-navigation 49 - -49
total -54

async chunk count

id before after diff
core 4 3 -1

ESLint disabled line counts

id before after diff
@kbn/core-chrome-browser-internal 7 6 -1
@kbn/shared-ux-chrome-navigation 2 - -2
total -3

Total ESLint disabled count

id before after diff
@kbn/core-chrome-browser-internal 7 6 -1
@kbn/shared-ux-chrome-navigation 2 - -2
total -3

History

Dosant added a commit that referenced this pull request Nov 25, 2025
## Summary

Follow up to #241114

This time we cleaning up everything related to the navigation trees
structure that is no longer needed in new nav

- [x] Platform @Dosant 
- [x] Search @Dosant (@TattdCodeMonkey to review)
- [x] Oblt @justinkambic 
- [x] Security @ashokaditya 

Guidance: 

- Use TypeScript errors to see what needs to be cleaned up:
- All V1 property no longer needed. Please Review. V1 branches no longer
needed
 - V2 properties renamed: iconV2 -> icon; badgeTypeV2 -> badgeType
- Body and Footer no longer need intermediate root nodes! The nav nodes
should go directly under body: [/* nodes */ ], footer: [/* footer */]
- @justinkambic, @ashokaditya, use search trees as an example. I cleaned
it up.

---------

Co-authored-by: Justin Kambic <jk@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Ashokaditya <ashokaditya@elastic.co>
umbopepato pushed a commit to umbopepato/kibana that referenced this pull request Nov 25, 2025
## Summary

Follow up to elastic#241114

This time we cleaning up everything related to the navigation trees
structure that is no longer needed in new nav

- [x] Platform @Dosant 
- [x] Search @Dosant (@TattdCodeMonkey to review)
- [x] Oblt @justinkambic 
- [x] Security @ashokaditya 

Guidance: 

- Use TypeScript errors to see what needs to be cleaned up:
- All V1 property no longer needed. Please Review. V1 branches no longer
needed
 - V2 properties renamed: iconV2 -> icon; badgeTypeV2 -> badgeType
- Body and Footer no longer need intermediate root nodes! The nav nodes
should go directly under body: [/* nodes */ ], footer: [/* footer */]
- @justinkambic, @ashokaditya, use search trees as an example. I cleaned
it up.

---------

Co-authored-by: Justin Kambic <jk@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Ashokaditya <ashokaditya@elastic.co>
seanrathier pushed a commit to seanrathier/kibana that referenced this pull request Nov 25, 2025
## Summary

Follow up to elastic#241114

This time we cleaning up everything related to the navigation trees
structure that is no longer needed in new nav

- [x] Platform @Dosant 
- [x] Search @Dosant (@TattdCodeMonkey to review)
- [x] Oblt @justinkambic 
- [x] Security @ashokaditya 

Guidance: 

- Use TypeScript errors to see what needs to be cleaned up:
- All V1 property no longer needed. Please Review. V1 branches no longer
needed
 - V2 properties renamed: iconV2 -> icon; badgeTypeV2 -> badgeType
- Body and Footer no longer need intermediate root nodes! The nav nodes
should go directly under body: [/* nodes */ ], footer: [/* footer */]
- @justinkambic, @ashokaditya, use search trees as an example. I cleaned
it up.

---------

Co-authored-by: Justin Kambic <jk@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Ashokaditya <ashokaditya@elastic.co>
eokoneyo pushed a commit to eokoneyo/kibana that referenced this pull request Dec 2, 2025
## Summary

Follow up to elastic#241114

This time we cleaning up everything related to the navigation trees
structure that is no longer needed in new nav

- [x] Platform @Dosant 
- [x] Search @Dosant (@TattdCodeMonkey to review)
- [x] Oblt @justinkambic 
- [x] Security @ashokaditya 

Guidance: 

- Use TypeScript errors to see what needs to be cleaned up:
- All V1 property no longer needed. Please Review. V1 branches no longer
needed
 - V2 properties renamed: iconV2 -> icon; badgeTypeV2 -> badgeType
- Body and Footer no longer need intermediate root nodes! The nav nodes
should go directly under body: [/* nodes */ ], footer: [/* footer */]
- @justinkambic, @ashokaditya, use search trees as an example. I cleaned
it up.

---------

Co-authored-by: Justin Kambic <jk@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Ashokaditya <ashokaditya@elastic.co>
JordanSh pushed a commit to JordanSh/kibana that referenced this pull request Dec 9, 2025
## Summary

Follow up to elastic#241114

This time we cleaning up everything related to the navigation trees
structure that is no longer needed in new nav

- [x] Platform @Dosant 
- [x] Search @Dosant (@TattdCodeMonkey to review)
- [x] Oblt @justinkambic 
- [x] Security @ashokaditya 

Guidance: 

- Use TypeScript errors to see what needs to be cleaned up:
- All V1 property no longer needed. Please Review. V1 branches no longer
needed
 - V2 properties renamed: iconV2 -> icon; badgeTypeV2 -> badgeType
- Body and Footer no longer need intermediate root nodes! The nav nodes
should go directly under body: [/* nodes */ ], footer: [/* footer */]
- @justinkambic, @ashokaditya, use search trees as an example. I cleaned
it up.

---------

Co-authored-by: Justin Kambic <jk@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Ashokaditya <ashokaditya@elastic.co>
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 release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants