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] Need a way to group nav items under a bold group title, without an accordion #167323

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

[Serverless/SideNav] Need a way to group nav items under a bold group title, without an accordion #167323

tsullivan opened this issue Sep 26, 2023 · 4 comments · Fixed by #169251
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

This issue is to resolve the structure we need for nav trees that use group titles.

Taken from: #164910 (review)

Where we currently have the bold title for groups of items (Search nav bar) [we still need to] make the "grouping" work with the breadcrumb by nesting the isGroupTitle node with its underlying children.

Explanation:
Where we have this design:
image

We need the internal state to be represented with a hierarchy that includes the title:

Home > Dev Tools > Console
Home > Dev Tools > Search Profiler
Home > Explore > Discover
Home > Explore > Dashboard
Home > Explore > Visualize Library
Home > Explore > Alerts
...etc

Currently, in the above design, Dev Tools, Explore, Content, and Security are "group titles" that themselves do not contain child items. We need a structure that can be used for the correct breadcrumbs.
@tsullivan tsullivan added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label Sep 26, 2023
@elasticmachine
Copy link
Contributor

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

@tsullivan tsullivan added the Project:Serverless Work as part of the Serverless project for its initial release label Sep 26, 2023
@cee-chen cee-chen mentioned this issue Sep 27, 2023
Merged
4 tasks
@cee-chen
Copy link
Contributor

How would y'all feel about an intermediary Kibana function that does something like this for "grouping":

elastic/eui@bc1e8a2

In the end, it still outputs a flattened array that EUI wants, but it should hopefully accomplish the nesting that Kibana needs for its architecture.

@tsullivan tsullivan mentioned this issue Sep 27, 2023
Merged
5 tasks
@sebelga
Copy link
Contributor

sebelga commented Sep 27, 2023

How would y'all feel about an intermediary Kibana function that does something like this for "grouping":

Something like that could work.

But if we take a step back, what we want is:

  • Group Navigation items
  • Have an optional title that renders on top of the group
  • If there is a title, have the option to mark the group as collapsible
  • If the group is marked as collapsible, indent the children to the right and add a vertical gray line on the left
  • If the group is not marked as collapsible, render the title in bold and do not indent the children

All of the above could be a single EUI component with the necessary props (probably just isCollapsible would be enough).

As you say, we can make it work in Kibana, but ideally it should come for free from EUI as I can see other consumers with the same kind of requirements. 👍

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]>
@sebelga
Copy link
Contributor

sebelga commented Oct 6, 2023

The Security solution nav also requires "grouping" (with no title) of different navigation nodes. When working on this issue we'll need to take it into account.

Screenshot 2023-10-06 at 15 28 39

@sebelga sebelga self-assigned this Oct 6, 2023
This was referenced Oct 13, 2023
Merged
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] Handle groups and vertical space sebelga/kibana
4 participants
@cee-chen @tsullivan @sebelga @elasticmachine