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] Expose options to add standard spacing between nav items. #167326

Closed
tsullivan opened this issue Sep 26, 2023 · 4 comments
Closed

[Serverless/SideNav] Expose options to add standard spacing between nav items. #167326

tsullivan opened this issue Sep 26, 2023 · 4 comments
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: https://github.com/elastic/kibana/pull/164910/files/87b8965b96ff38ebde7e21622e5515fb4e2b0db4#r1334685805

This issue is to add a simple prop for nav item declarations where we need vertical spacing between a previous "group."

[Options for] "small", "medium" and "large" should be enough.
@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)

@cee-chen
Copy link
Contributor

For EUI implementation, I'm currently leaning towards a wildcard renderItem: () => {} callback that you can pass to an subitem, e.g. items={[ { renderItem: SomeCustomSpacerComponent } ]}. That way Kibana can enforce its own spacer API as needed (and whatever else might be needed in the future).

Thoughts?

@cee-chen cee-chen mentioned this issue Sep 27, 2023
Merged
4 tasks
@tsullivan tsullivan mentioned this issue Sep 27, 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

@cee-chen sounds good to me. We could use <EuiSpacer /> as custom content returned from the renderItem to achieve what we need.

@sebelga
Copy link
Contributor

sebelga commented Oct 24, 2023

Closing this issue as it has been solved in #169251
There is a default space between level 1 groups (that can be overriden).
And a new spaceBefore prop has been added to control vertical spacing.

@sebelga sebelga closed this as completed Oct 24, 2023
@cee-chen cee-chen mentioned this issue Feb 5, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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

No branches or pull requests
4 participants
@cee-chen @tsullivan @sebelga @elasticmachine