Skip to content

Comments

[Solution Side Nav] Fix panel styles#219478

Merged
tsullivan merged 21 commits intoelastic:mainfrom
tsullivan:sidenav/panel-style-fixes
May 9, 2025
Merged

[Solution Side Nav] Fix panel styles#219478
tsullivan merged 21 commits intoelastic:mainfrom
tsullivan:sidenav/panel-style-fixes

Conversation

@tsullivan
Copy link
Member

@tsullivan tsullivan commented Apr 29, 2025

Summary

Epic: https://github.com/elastic/kibana-team/issues/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
  4. Fix position of accordion group arrows in the secondary panel (viewable only from Storybook)
  5. isOpen is not really needed and can be cleaned up
  6. (not panel-related) remove the on-hover and on-focus underline from nav items

Checklist from Kate:

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

Screenshots

Security Solution Serverless

secondary-panel-nav-fixes.mp4

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

@tsullivan tsullivan force-pushed the sidenav/panel-style-fixes branch 2 times, most recently from 34ebd43 to 6f5be50 Compare May 2, 2025 15:00
@tsullivan tsullivan marked this pull request as ready for review May 2, 2025 15:30
@tsullivan tsullivan requested review from a team as code owners May 2, 2025 15:30
@tsullivan tsullivan added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels May 2, 2025
* Note: this property is currently only used for (1) "group" nodes and (2) in the navigation
* panel opening on the right of the side nav.
*/
appendHorizontalRule?: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

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

Horizontal rules will be automatically added in the panel in-between each group

@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. labels May 2, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2025

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@tsullivan tsullivan force-pushed the sidenav/panel-style-fixes branch from 6f5be50 to d43557e Compare May 2, 2025 15:33
Copy link
Contributor

@ek-so ek-so left a comment

Choose a reason for hiding this comment

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

Hey Tim! As we discussed:
image

@tsullivan tsullivan force-pushed the sidenav/panel-style-fixes branch from 3e596e2 to 56c6918 Compare May 7, 2025 00:28
@tsullivan tsullivan requested a review from a team as a code owner May 7, 2025 00:28
Copy link
Contributor

@ek-so ek-so left a comment

Choose a reason for hiding this comment

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

As discussed — alignment for the text inside the 2nd panel + blue color for selected items. @tsullivan and thank you again for all those changes :)

@tsullivan tsullivan added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label May 7, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Copy link
Contributor

@kapral18 kapral18 left a comment

Choose a reason for hiding this comment

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

LGTM for explore

Copy link
Member

@maryam-saeidi maryam-saeidi left a comment

Choose a reason for hiding this comment

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

defaultMessage changes LGTM

@tsullivan tsullivan requested a review from a team as a code owner May 8, 2025 16:59
@tsullivan tsullivan removed the request for review from a team May 8, 2025 20:08
@tsullivan tsullivan enabled auto-merge (squash) May 9, 2025 14:48
@elasticmachine
Copy link
Contributor

⏳ Build in-progress

  • Buildkite Build
  • Commit: 5b3498b
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-219478-5b3498b7ecd8

Failed CI Steps

History

@tsullivan tsullivan merged commit 0fcc9a3 into elastic:main May 9, 2025
11 checks passed
@tsullivan tsullivan deleted the sidenav/panel-style-fixes branch May 9, 2025 17:39
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#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
@tsullivan
Copy link
Member Author

This PR exists in 8.19 as well.

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 ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants