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.

Sign up for GitHub

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

Jump to bottom

[Serverless/SideNav] Sub-item accordions should not kept open, should be collapsed by default #167328

Closed
tsullivan opened this issue Sep 26, 2023 · 4 comments · Fixed by #169651
Closed

[Serverless/SideNav] Sub-item accordions should not kept open, should be collapsed by default #167328

tsullivan opened this issue Sep 26, 2023 · 4 comments · Fixed by #169651
Assignees
@sebelga
Labels
Project:Serverless Work as part of the Serverless project for its initial release Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)

Comments

@tsullivan
Copy link
Member

Taken from #164910 (comment)

Certain sections in a side nav should have an "accordion" treatment: they should be collapsible and show the arrow.

As of the latest updates to the components used for side nav, we have an intermediate state: the arrows for these sections are not shown, and the child items are not collapsible. To resolve this, we need proper state management that keeps the necessary accordions expanded. The side nav component should manage state about:

  1. Which sections should be expanded, based on the current isSelected nav item
  2. Which sections have been expanded by user clicks and should not be automatically collapsed
  3. All other collapsible sections can be automatically collapsed.
@tsullivan tsullivan added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Project:Serverless Work as part of the Serverless project for its initial release labels Sep 26, 2023
@elasticmachine
Copy link
Contributor

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

@tsullivan tsullivan mentioned this issue Sep 26, 2023
Merged
5 tasks
Ikuni17 pushed a commit that referenced this issue Sep 27, 2023
@tsullivan @sebelga
Project Side Navigation: Use EuiCollapsibleNavBeta component (#164910)
0c680d7 
## Summary

Closes #162507
Relates to #166545
^ additional IA-related tasks - related to the alignment discussions -
can be found here

## Work for next steps
In this PR, some work items are being saved for a next PR:
1. _Only affects Search solution_: Navigation "group titles" do not
create a breadcrumb item, as sub-items in the group are not
hierarchically under the title. To address this, group titles may be
going away from the design.
#167323
2. _Only affects Observability solution_: Navigation accordions can not
be collapsed and do not show arrow icons. To address this, in a later PR
we will add internal state management for the open/closed state of each
accordion. #167328
3. _Affects all solutions:_ The "collapsed" state of the side nav should
show a docked view with icons-only. To address this, in later PRs we
will bring Security solution into the unified nav components.
4. #167326
5. #167330
6. #167332

### Recordings
These videos show a before-and-after with the new UI. 
| project | old | new |
|--|--|--|
|observability|
https://github.com/elastic/kibana/assets/908371/663765a3-4e4b-416e-b7d5-7d87eece83e8
| <img width="298" alt="CleanShot 2023-09-22 at 14 20 48@2x"
src="https://github.com/elastic/kibana/assets/446285/d61f6fe0-a6a9-4806-bc27-08b0ff2afb49">
|
|search|
https://github.com/elastic/kibana/assets/908371/f383773e-27a8-4485-8289-274d8231b960
| <img width="281" alt="CleanShot 2023-09-22 at 14 18 43@2x"
src="https://github.com/elastic/kibana/assets/446285/901b8fc1-9945-4cee-b566-5e4539f08043">
|
|security|
https://github.com/elastic/kibana/assets/908371/481f4533-64e5-41db-bc8e-5012f82c188a
| *will change to the new style after this PR and the flyout/panel
support are completed |

### Checklist

- [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
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

---------

Co-authored-by: Sébastien Loix <[email protected]>
@tsullivan
Copy link
Member Author

We're giving this issue low priority, since our designs will not use accordions for sub-items in the near future.

@sebelga
Copy link
Contributor

sebelga commented Oct 16, 2023

@tsullivan I was thinking in putting back the accordion and handle their collapsed state based on the URL location once #167774 is merged. Are you saying that we won't want to support accordions anymore in the left navigation and I shouldn't dedicate time on making them work?

cc @sixstringcode @ryankeairns

@sebelga
Copy link
Contributor

sebelga commented Oct 25, 2023

Discard my previous comment. I fixed the active state in #169651

@sebelga sebelga mentioned this issue Oct 25, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sebelga sebelga

Labels
Project:Serverless Work as part of the Serverless project for its initial release Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Serverless nav] Accordion auto-expand state + polish work sebelga/kibana
3 participants
@tsullivan @sebelga @elasticmachine