Skip to content

Comments

[Solution Side Nav] Remove PanelContentProvider & support optional title in nav node#218156

Merged
tsullivan merged 2 commits intoelastic:mainfrom
tsullivan:sidenav/remove-panel-content-provider-support
Apr 15, 2025
Merged

[Solution Side Nav] Remove PanelContentProvider & support optional title in nav node#218156
tsullivan merged 2 commits intoelastic:mainfrom
tsullivan:sidenav/remove-panel-content-provider-support

Conversation

@tsullivan
Copy link
Member

@tsullivan tsullivan commented Apr 14, 2025

Epic: https://github.com/elastic/kibana-team/issues/1439
Needed for #218050 (adjustments to types for title field in ChromeProjectNavigationNode)

Summary

  1. PanelContentProvider was used for security solution, but is no longer used. This removes it to simplify the interfaces for panel .
  2. Allow title of navGroup to be optional. This allows the correct design for nav items in the footer, which are child-items of a nav group with no title

Progress towards fixing footer issues

The look and feel of nav items in the footer was wrong. Now that navGroup titles are optional, we can make footer navigation items appear more like the intended design. Run the yarn storybook shared_ux and view the Chrome Navigation page for the proper way footer nav items should look.

Storybook before Storybook after
image image

To achieve this fix within Solutions, teams will need to update their navigation structure and move the top-level footer items into a single navGroup.

Example code fix

File: x-pack/solutions/observability/plugins/observability/public/navigation_tree.ts

Explanation:
* Structure the footer with a single `navGroup` containing the nav footer contents as sub-items
   * i.e., replace the structure of: `navItem / navItem / navGroup`
* Do not add a `title` for this `navGroup`
* For Stack Management, use `renderAs: 'accordion'` and `spaceBefore: null`
* Remove `recentlyAccessed` for now

@@ -325,9 +325,10 @@ function createNavTree({ streamsAvailable }: { streamsAvailable?: boolean }) {
       },
     ],
     footer: [
-      { type: 'recentlyAccessed' },
       {
-        type: 'navItem',
+        type: 'navGroup',
+        id: 'observability_project_nav_footer',
+        children: [
+          {
             title: i18n.translate('xpack.observability.obltNav.addData', {
               defaultMessage: 'Add data',
             }),
@@ -335,7 +336,6 @@ function createNavTree({ streamsAvailable }: { streamsAvailable?: boolean }) {
             icon: 'launch',
           },
           {
-        type: 'navItem',
             id: 'devTools',
             title: i18n.translate('xpack.observability.obltNav.devTools', {
               defaultMessage: 'Developer tools',
@@ -344,13 +344,14 @@ function createNavTree({ streamsAvailable }: { streamsAvailable?: boolean }) {
             icon: 'editorCodeBlock',
           },
           {
-        type: 'navGroup',
             id: 'project_settings_project_nav',
             title: i18n.translate('xpack.observability.obltNav.management', {
               defaultMessage: 'Management',
             }),
             icon: 'gear',
             breadcrumbStatus: 'hidden',
+            renderAs: 'accordion',
+            spaceBefore: null,
             children: [
               {
                 id: 'stack_management',
@@ -455,6 +456,8 @@ function createNavTree({ streamsAvailable }: { streamsAvailable?: boolean }) {
             ],
           },
         ],
+      },
+    ],
   };

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

@tsullivan tsullivan force-pushed the sidenav/remove-panel-content-provider-support branch from f823a8e to 9a88105 Compare April 14, 2025 17:49
@tsullivan tsullivan marked this pull request as ready for review April 14, 2025 22:37
@tsullivan tsullivan requested a review from a team as a code owner April 14, 2025 22:37
@tsullivan tsullivan added Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// backport:version Backport to applied version labels v9.1.0 v8.19.0 labels Apr 14, 2025
@elasticmachine
Copy link
Contributor

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

path: 'group1.item1',
},
],
] as ChromeProjectNavigationNode[],
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not entirely sure why this was necessary 🤔

@tsullivan tsullivan marked this pull request as draft April 14, 2025 22:43
@tsullivan tsullivan added the release_note:skip Skip the PR/issue when compiling release notes label Apr 14, 2025
@tsullivan tsullivan force-pushed the sidenav/remove-panel-content-provider-support branch from 71710bf to f471af8 Compare April 14, 2025 23:23
{
id: 'footer-section5',
title: 'Parent item, closed',
id: 'example_project_footer',
Copy link
Member Author

Choose a reason for hiding this comment

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

footer should have a single group of sub-items, with no title for that group

@tsullivan tsullivan force-pushed the sidenav/remove-panel-content-provider-support branch from f471af8 to 3cda9eb Compare April 15, 2025 01:28
@tsullivan tsullivan marked this pull request as ready for review April 15, 2025 04:34
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

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/shared-ux-chrome-navigation 29 26 -3
serverless 24 23 -1
total -4

Async chunks

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

id before after diff
navigation 18.9KB 18.6KB -296.0B
serverless 19.6KB 19.3KB -296.0B
total -592.0B

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 872 873 +1

Page load bundle

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

id before after diff
core 433.2KB 433.2KB +6.0B
navigation 12.8KB 12.7KB -92.0B
serverless 9.7KB 9.6KB -46.0B
total -132.0B
Unknown metric groups

API count

id before after diff
@kbn/shared-ux-chrome-navigation 38 35 -3
serverless 24 23 -1
total -4

History

@tsullivan tsullivan merged commit b9c2b57 into elastic:main Apr 15, 2025
11 checks passed
@tsullivan tsullivan deleted the sidenav/remove-panel-content-provider-support branch April 15, 2025 15:04
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/14472802881

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 218156

Questions ?

Please refer to the Backport tool documentation

@tsullivan
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

tsullivan added a commit to tsullivan/kibana that referenced this pull request Apr 15, 2025
…tle in nav node (elastic#218156)

Epic: elastic/kibana-team#1439
Needed for elastic#218050 (adjustments to
types for `title` field in `ChromeProjectNavigationNode`)

## Summary

1. `PanelContentProvider` was used for security solution, but is no
longer used. This removes it to simplify the interfaces for panel .
2. Allow title of `navGroup` to be optional. This allows the correct
design for nav items in the footer, which are child-items of a nav group
with no title

## Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [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

(cherry picked from commit b9c2b57)
tsullivan added a commit that referenced this pull request Apr 16, 2025
…nal title in nav node (#218156) (#218330)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Solution Side Nav] Remove PanelContentProvider & support optional
title in nav node
(#218156)](#218156)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-04-15T15:04:01Z","message":"[Solution
Side Nav] Remove PanelContentProvider & support optional title in nav
node (#218156)\n\nEpic:
https://github.com/elastic/kibana-team/issues/1439\nNeeded for
#218050 (adjustments to\ntypes for
`title` field in `ChromeProjectNavigationNode`)\n\n## Summary\n\n1.
`PanelContentProvider` was used for security solution, but is no\nlonger
used. This removes it to simplify the interfaces for panel .\n2. Allow
title of `navGroup` to be optional. This allows the correct\ndesign for
nav items in the footer, which are child-items of a nav group\nwith no
title\n\n## Checklist\n\nCheck the PR satisfies following conditions.
\n\nReviewers should verify this PR satisfies this list as well.\n\n-
[x]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas
added for features that require explanation or tutorials\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"b9c2b57c23d1d74d4fef025d64b820216ddce272","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:SharedUX","backport:version","v9.1.0","v8.19.0"],"title":"[Solution
Side Nav] Remove PanelContentProvider & support optional title in nav
node","number":218156,"url":"https://github.com/elastic/kibana/pull/218156","mergeCommit":{"message":"[Solution
Side Nav] Remove PanelContentProvider & support optional title in nav
node (#218156)\n\nEpic:
https://github.com/elastic/kibana-team/issues/1439\nNeeded for
#218050 (adjustments to\ntypes for
`title` field in `ChromeProjectNavigationNode`)\n\n## Summary\n\n1.
`PanelContentProvider` was used for security solution, but is no\nlonger
used. This removes it to simplify the interfaces for panel .\n2. Allow
title of `navGroup` to be optional. This allows the correct\ndesign for
nav items in the footer, which are child-items of a nav group\nwith no
title\n\n## Checklist\n\nCheck the PR satisfies following conditions.
\n\nReviewers should verify this PR satisfies this list as well.\n\n-
[x]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas
added for features that require explanation or tutorials\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"b9c2b57c23d1d74d4fef025d64b820216ddce272"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/218156","number":218156,"mergeCommit":{"message":"[Solution
Side Nav] Remove PanelContentProvider & support optional title in nav
node (#218156)\n\nEpic:
https://github.com/elastic/kibana-team/issues/1439\nNeeded for
#218050 (adjustments to\ntypes for
`title` field in `ChromeProjectNavigationNode`)\n\n## Summary\n\n1.
`PanelContentProvider` was used for security solution, but is no\nlonger
used. This removes it to simplify the interfaces for panel .\n2. Allow
title of `navGroup` to be optional. This allows the correct\ndesign for
nav items in the footer, which are child-items of a nav group\nwith no
title\n\n## Checklist\n\nCheck the PR satisfies following conditions.
\n\nReviewers should verify this PR satisfies this list as well.\n\n-
[x]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas
added for features that require explanation or tutorials\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"b9c2b57c23d1d74d4fef025d64b820216ddce272"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
tsullivan added a commit that referenced this pull request May 2, 2025
## Summary

Epic: elastic/kibana-team#1439
~Depends on #218156

Addresses footer issues and a few content issues


https://github.com/user-attachments/assets/07063aa7-8a49-4cb5-9fd2-a6e1cbd674b7


1. Footer item texts should not be bold
2. We should have the spacing of regular nav sub-items in the footer

Other fixes:
1. Fix text casing in "Machine Learning" (Note: [other casing issues
exist](#217352))
2. Remove `recentlyAccessed` sections

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] 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)
- [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
tsullivan added a commit that referenced this pull request May 9, 2025
## Summary

Epic: elastic/kibana-team#1439
Depends on #218513

### List of changes
1. Match header text style with the main panel
2. Match spacing between items with the main panel
3. Show horizontal line between groups in the panel
- This restores a custom error that was in the code prior to #218156,
but adds better handling and tests to the error
5. Fix position of accordion group arrows in the secondary panel
(viewable only from Storybook)
6. isOpen is not really needed and can be cleaned up
- (see
https://github.com/elastic/kibana/pull/217708/files#diff-9e95eb3e4be14fb61caca86b48be4f03ca63393eeeadcd0b36f7371958a7e5e1L55)
7. (not panel-related) remove the on-hover and on-focus underline from
nav items

### Checklist from Kate:
- [x] text font-weight of solution title in main panel
- [x] text font-weight of secondary panel link content
- [x] zero space between items
- [x] use ~~12px~~ nice-looking margin from the link to the edge of the
panel
- [x] ensure the divider reaches to the edge of the panel
- [x] increase space from panel header to the items
- [x] ensure the width of both panels is 248px
- [x] secondary panel header should vertically align with the below nav
items
- [x] (not panel-related) font color and icon color of selected items
should be "primary" blue

### Screenshots

**Security Solution Serverless**


https://github.com/user-attachments/assets/83ca1c25-f44a-4270-96a8-e28e7b6d7041

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] 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)
- [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
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
## Summary

Epic: elastic/kibana-team#1439
~Depends on elastic#218156

Addresses footer issues and a few content issues


https://github.com/user-attachments/assets/07063aa7-8a49-4cb5-9fd2-a6e1cbd674b7


1. Footer item texts should not be bold
2. We should have the spacing of regular nav sub-items in the footer

Other fixes:
1. Fix text casing in "Machine Learning" (Note: [other casing issues
exist](elastic#217352))
2. Remove `recentlyAccessed` sections

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] 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)
- [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
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
## Summary

Epic: elastic/kibana-team#1439
Depends on elastic#218513

### List of changes
1. Match header text style with the main panel
2. Match spacing between items with the main panel
3. Show horizontal line between groups in the panel
- This restores a custom error that was in the code prior to elastic#218156,
but adds better handling and tests to the error
5. Fix position of accordion group arrows in the secondary panel
(viewable only from Storybook)
6. isOpen is not really needed and can be cleaned up
- (see
https://github.com/elastic/kibana/pull/217708/files#diff-9e95eb3e4be14fb61caca86b48be4f03ca63393eeeadcd0b36f7371958a7e5e1L55)
7. (not panel-related) remove the on-hover and on-focus underline from
nav items

### Checklist from Kate:
- [x] text font-weight of solution title in main panel
- [x] text font-weight of secondary panel link content
- [x] zero space between items
- [x] use ~~12px~~ nice-looking margin from the link to the edge of the
panel
- [x] ensure the divider reaches to the edge of the panel
- [x] increase space from panel header to the items
- [x] ensure the width of both panels is 248px
- [x] secondary panel header should vertically align with the below nav
items
- [x] (not panel-related) font color and icon color of selected items
should be "primary" blue

### Screenshots

**Security Solution Serverless**


https://github.com/user-attachments/assets/83ca1c25-f44a-4270-96a8-e28e7b6d7041

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] 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)
- [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
qn895 pushed a commit to qn895/kibana that referenced this pull request Jun 3, 2025
## Summary

Epic: elastic/kibana-team#1439
~Depends on elastic#218156

Addresses footer issues and a few content issues


https://github.com/user-attachments/assets/07063aa7-8a49-4cb5-9fd2-a6e1cbd674b7


1. Footer item texts should not be bold
2. We should have the spacing of regular nav sub-items in the footer

Other fixes:
1. Fix text casing in "Machine Learning" (Note: [other casing issues
exist](elastic#217352))
2. Remove `recentlyAccessed` sections

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] 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)
- [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
qn895 pushed a commit to qn895/kibana that referenced this pull request Jun 3, 2025
## Summary

Epic: elastic/kibana-team#1439
Depends on elastic#218513

### List of changes
1. Match header text style with the main panel
2. Match spacing between items with the main panel
3. Show horizontal line between groups in the panel
- This restores a custom error that was in the code prior to elastic#218156,
but adds better handling and tests to the error
5. Fix position of accordion group arrows in the secondary panel
(viewable only from Storybook)
6. isOpen is not really needed and can be cleaned up
- (see
https://github.com/elastic/kibana/pull/217708/files#diff-9e95eb3e4be14fb61caca86b48be4f03ca63393eeeadcd0b36f7371958a7e5e1L55)
7. (not panel-related) remove the on-hover and on-focus underline from
nav items

### Checklist from Kate:
- [x] text font-weight of solution title in main panel
- [x] text font-weight of secondary panel link content
- [x] zero space between items
- [x] use ~~12px~~ nice-looking margin from the link to the edge of the
panel
- [x] ensure the divider reaches to the edge of the panel
- [x] increase space from panel header to the items
- [x] ensure the width of both panels is 248px
- [x] secondary panel header should vertically align with the below nav
items
- [x] (not panel-related) font color and icon color of selected items
should be "primary" blue

### Screenshots

**Security Solution Serverless**


https://github.com/user-attachments/assets/83ca1c25-f44a-4270-96a8-e28e7b6d7041

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] 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)
- [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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants