Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Solution nav] Prevent href click on accordion button #183508

Merged

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented May 15, 2024

In this PR I've added support for href value to the accordion header button required to fix https://github.com/elastic/observability-serverless/issues/24.

With the changes of this PR it is now possible to declare a link to an accordion parent, ant not have it affect the accordion toggle on/off behaviour. The link is then only used for the Breadcrumb.

Manual testing

To test manually the change, apply the diff from here https://github.com/elastic/kibana/pull/183189/files and launch serverless observability. You should have the breadcrumb parent.

Screenshot 2024-05-15 at 14 38 15

@sebelga
Copy link
Contributor Author

sebelga commented May 15, 2024

/ci

@sebelga sebelga added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Feature:Chrome Core's Chrome UI (sidenav, header, breadcrumbs) labels May 15, 2024
@sebelga sebelga marked this pull request as ready for review May 15, 2024 13:41
@sebelga sebelga requested a review from a team as a code owner May 15, 2024 13:41
@elasticmachine
Copy link
Contributor

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

@neptunian
Copy link
Contributor

Tested locally and works. I was able to get the tests to pass as well by using navId here after giving the child link an ID to distinguish it from the parent link id. Thank you!

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

code lgtm

@sebelga sebelga enabled auto-merge (squash) May 16, 2024 10:08
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
navigation 22.0KB 22.0KB +34.0B
serverless 22.2KB 22.3KB +34.0B
total +68.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@sebelga sebelga merged commit ee85270 into elastic:main May 16, 2024
18 checks passed
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels May 16, 2024
@sebelga sebelga deleted the solution-sidenav/link-breadcrumb-accordion branch May 16, 2024 11:40
neptunian added a commit that referenced this pull request May 16, 2024
## Summary

Currently the breadcrumb of the parent navigation item isn't a link:

<img width="339" alt="Screenshot 2024-05-10 at 2 06 42 PM"
src="https://github.com/elastic/kibana/assets/1676003/1471125f-59d7-468e-8ede-3a3c3c3e42dc">

This change makes it so the breadcrumb of parent navigation item is
linkable:
<img width="293" alt="Screenshot 2024-05-10 at 2 21 27 PM"
src="https://github.com/elastic/kibana/assets/1676003/aad5e650-33e1-419d-b866-0b7ec70e1f42">

The links will go to the first item in the list of child links.

The merging of this PR depends on
#183508 allows for this
functionality to work without making the left side parent collapsible
link, also a href link - only the breadcrumb.

## Testing
- Start a serverless kibana instance

- Navigate to a child link of an accordion using the left side
navigation
<img width="589" alt="Screenshot 2024-05-15 at 11 26 24 AM"
src="https://github.com/elastic/kibana/assets/1676003/d225967d-37d1-4f19-b3f5-856fcc1bce68">

- The breadcrumb should of this page should be a link and link you to
the first child link of the accordion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Chrome Core's Chrome UI (sidenav, header, breadcrumbs) release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants