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

Updates to navigation components in support of Kibana nav redesign #6759

Closed
10 tasks
ryankeairns opened this issue May 9, 2023 · 11 comments · Fixed by #7034
Closed
10 tasks

Updates to navigation components in support of Kibana nav redesign #6759

ryankeairns opened this issue May 9, 2023 · 11 comments · Fixed by #7034
Assignees
Labels

Comments

@ryankeairns
Copy link
Contributor

ryankeairns commented May 9, 2023

As part of the upcoming initiatives, we were tasked with redesigning Kibana's main navigation to accommodate new workstream-driven info architectures (IA). This redesigned navigation aims to unify the existing main and solution-level navigations into a singular navigation. Here are links to our latest designs:

The Platform UX team's proposal largely utilizes functionality from the EuiCollapsibleNav and EuiSideNav components, but it does deviate in a number of key ways as outlined below.

Implementation considerations

Before getting into the design details, there is an open question as to how we proceed with the implementation. Some options that come to mind are:

  1. Make no changes to the EUI components, and rely on the shared UX team to override styles and functionality and/or create custom components as needed.
  2. Make changes to the EUI components on main.
  3. Current thinking 👉 Utilize a feature branch and ship changes as a separate package on npm.
  4. Add props with options that accommodate for the differences.
  5. Create an entirely new EUI component.

Depending on how we split the work in this issue, we may find that the visual design changes, for example, are safe to push to main. That said, it would be helpful to see them in the flesh and try things out in Kibana before committing to main. With this bit of uncertainty, option 3 seems a good path for trying things on in a low-risk manner.

Currently, there are two nav versions in Kibana. One for the current design and a second that is the beginning of this new design (emphasis on chrome service/API not UI design changes). It may be that Shared UX (i.e. Kibana Eng) want to apply this new/alternative package to only the latter for initial launch.

Suggestion

  1. Start with the Emotion conversion for these components
  2. Tackle the visual design changes together
  3. Proceed to functional enhancements

For item 3, the right-aligned icon is likely to be straightforward and relatively small. The new collapsible design will be the largest of all items in this effort and needs further discussion. Given that, I will mark it as hold until we get final alignment from our Product and UX peers.

Design changes

The sections below provide a component-level comparison of key deviations between the existing EUI components and the newly proposed navigation redesign. At a high level, the tasks can be summarized as follows:

Tasks - Visual design changes

Preview Give feedback

Tasks - Functional enhancements

Preview Give feedback

EuiCollapsibleNav Differences

Functional enhancements

  • EuiCollapsibleNav currently offers a docked option, which allows the navigation to operate similar to a push flyout, where the navigation occupies and shares space with the adjacent body content (rather than being overlaid). As the new navigation designs are planning to absorb the responsibilities of both the current Kibana main navigation and solution-level navigations, we proposed that the new navigation design be an ever-present fixture on the page (almost as if it were permanently docked).

    • In order to give the user control over how much horizontal real estate this ever-present navigation occupies, we wish to allow them to toggle between a collapsed icon-only version of the navigation (the default setting) and a larger expanded navigation.

    image

    • In order to allow users to continue to interact with the navigation in the redesigned navigation's proposed collapsed size, we desire the ability to offer an alternative EuiContextMenu-based navigation, which would utilize nested menus to mimic the nested accordions in the larger expanded navigation designs.

    image

Visual changes

  • EuiCollapsibleNavGroup components appear to have a top border applied to visually separate siblings. The navigation redesign proposes we omit these borders to cut down on the visual noise in situations where many navigation groups are present.

    image

  • EuiCollapsibleNavGroup button styles in the redesigned navigation propose different hover and current state styles, different padding, and different spacing between icon and title.

    image

  • The current accordion arrow button within EuiCollapsibleNavGroup appears to take an arrowRight icon and rotate it 90 degrees to achieve the collapsed/expanded accordion effect. While this effect works great for left-aligned accordion arrows, it feels unexpected when right-aligned (which causes the collapsed right-facing arrow to look more like it will open a menu or navigate you forward, rather than open an accordion). The redesigned navigation also uses right-aligned arrows to indicate the accordion state, but instead of directing them right/down, they are down/up (better indicating that the panel will be sliding down or up with the accordion behavior). The designs also propose a smaller arrow icon (12px) and a low contrast color for the arrow when the accordion is closed to avoid visually overwhelming users if a lot of accordions are visible.

    image

EuiSideNav Differences

Functional enhancements

  • For non-accordion EuiSideNavItem components, the new designs account for the possibility of a right-aligned appended icon. Specifically, this will be used for the low contrast 12px icon indicating an external links in the navigation.

    image

Visual changes

  • EuiSideNavItem styles in the redesigned navigation propose different hover and current state styles, and different padding.

    image

  • The current accordion arrow icon that conditionally appears within EuiSideNavItem appears to use hard-coded arrowRight and arrowDown icons to achieve the collapsed/expanded accordion effect. For the same reasons I detail in the EuiCollapsibleNavGroup accordion comments above, the redesigned navigation suggests using arrowDown and arrowUp icons instead. The designs also propose a low contrast color for the arrow when the accordion is closed to avoid visually overwhelming users if a lot of accordions are visible.

    image

  • When a EuiSideNavItem component with nested items is expanded, we currently show prepended vertical lines and horizontal ticks to help users better visualize the hierarchy of the items in the list. The redesigned navigation maintains this general concept, but alters it slightly by removing the horizontal ticks altogether (to reduce visual noise) and only using the vertical lines to indicate hierarchy. It also aligns the left side of the vertical line with the left edge of the parent text (rather than being kicked in 8px).

  • Nested EuiSideNavItem text receives a subdued color currently. In the redesigned navigation, we're proposing that these nested items maintain the standard text color.

  • When a EuiSideNavItem component with nested items is expanded, the redesigned navigation proposes adding an additional 16px of spacing at the bottom of the opened accordion to provide addition spacing between the end of the accordion's contents and the next item in the list.


EuiBreadcrumbs Differences

➡️ Moved breadcrumb work to #7015

Functional enhancements

  • As part of our design explorations in the main navigation, we played with a few possibilities for where to place project links, cloud deployment links, and the space selector. We ultimately landed on the idea to prepend them as part of the breadcrumbs, as that fit very well hierarchically. Ideally, we'd like the ability to have these prepended breadcrumb items that will trigger a popover when clicked.

    image

CCing @tsullivan in case there's anything we missed.

@tsullivan
Copy link
Member

Are there changes that need to go into EUI components as part of the new header for serverless projects?

Specifically, creating the nav show/hide button looks like it is to be done with EuiHeaderSectionItem, but will have a box-like border. Is it best to implement that in the Kibana side with custom code, that uses the correct theme color for the border, or should the EUI provide a way to make that box-like header section item?

@cee-chen
Copy link
Contributor

Is it best to implement that in the Kibana side with custom code, that uses the correct theme color for the border, or should the EUI provide a way to make that box-like header section item?

EUI should be handling this on our end.

@cee-chen
Copy link
Contributor

cee-chen commented Jun 20, 2023

Current thinking 👉 Utilize a feature branch and ship changes as a separate package on npm.
@ryankeairns My strong vote would be to create a new internal (not a public top-level exported) component called EuiCollapsibleNavBeta that merges to main and can be imported in Kibana by diving into the EUI library specifically, e.g.

import { EuiCollapsibleNavBeta } from '@elastic/eui/lib/components/collapsible_nav/collapsible_nav_beta';

Using this approach means significantly less headache with npm exports and staying up to date on main (especially since we have several tech debt items such as node/React versions occurring) while still allowing us to remain flexible and tweak the component as-needed if specific props aren't working as expected.

@ryankeairns
Copy link
Contributor Author

Roger that regarding an internal component.

On the topic of top header, I suspect we need a separate issue to capture the collapse button design. I don't know that there are other new changes (perhaps breadcrumbs 🤔 ), but I will suss those out.

cc:/ @MichaelMarcialis

@MichaelMarcialis
Copy link
Contributor

MichaelMarcialis commented Jun 23, 2023

I don't know that there are other new changes (perhaps breadcrumbs 🤔 ), but I will suss those out.

@ryankeairns: Aside from what is mentioned in the original comment above (re: inclusion of deployment/project and space as part of breadcrumbs), I don't believe the current designs suggest any changes to breadcrumbs other than to use the type="page" prop to give it that more subdued styling.

The header remains largely the same for both expanded and collapsed navigation modes in larger viewport sizes. The only difference in the two is the icon being used to represent the collapse/expand action.

Where the header (and navigation) does start to exhibit some more significant change is at smaller viewport sizes (see Figma examples).

@cee-chen
Copy link
Contributor

Michael, you're a mind-reader! I was just about to ask for mobile views :)

@cee-chen
Copy link
Contributor

cee-chen commented Jul 6, 2023

So I ended up going with option 5 (a brand new component called EuiCollapsibleNavItem that's an amalgamation of EuiCollapsibleNavGroup and EuiSideNavItem). I have an MVP of the new design that you can check out locally here in Storybook: #6904 (see QA steps).

There are a couple UX questions I wanted to iron out before writing more tests and un-drafting the PR, such as:

  1. Whether items can be both a link to a page and an accordion toggle - I assumed yes and based most of my work around that given that's how Kibana currently operates
  2. What level of nesting we're expecting (I see Observability has a 3rd level in the Figma mocks but not a mock for what's inside there)
  3. Whether subitems should show isSelected state
  4. Whether EUI should attempt to implement the noted behavior (only 1 accordion open at a time) by default or leave that up to Kibana

@tsullivan, in particular I'd love to chat at next week's meeting about the new API and confirm that it'll work for your use case / doesn't have any missing props / won't be too annoying to use or migrate to.

Thanks y'all!

@ryankeairns
Copy link
Contributor Author

Looking forward to the review. Looking really good!

@MichaelMarcialis
Copy link
Contributor

Hey, @cee-chen. Here are some quick answers to your questions before our review today.

Whether items can be both a link to a page and an accordion toggle - I assumed yes and based most of my work around that given that's how Kibana currently operates

We spoke about this in Slack, but I'll paste my response here as well for visibility. My original intent for root level navigation items was that they could function as either an accordion or a link, but not both (to keep it simple and predictable for users). So in the case of the solutions (like Observability and Security), their root level item was intended to function only as an accordion toggle. That said, I know you mentioned having implemented it with the possibility for both, so we can evaluate further in our review.

What level of nesting we're expecting (I see Observability has a 3rd level in the Figma mocks but not a mock for what's inside there)

Correct. Based on the current designs there could be a maximum of three nested accordion levels. However, the navigation items and their location/depth is changing on a daily basis at the moment while the solutions are ironing things out. While I can't guarantee it will be more or less than that, my hope is that we never or rarely go beyond a third level, unless absolutely necessary.

Whether subitems should show isSelected state

Yes. I've added a quick example of the selected state in various levels and accordion states below. The thinking is that each individual item should be able to support a selected state. If the currently selected item has had its parent accordion collapsed by the user, that parent should be given the selected state while collapsed.

image

Whether EUI should attempt to implement the noted behavior (only 1 accordion open at a time) by default or leave that up to Kibana

Good question. If there are style implications involved when determining if the navigation should allow only one or all accordions to be opened simultaneously (as I suspect there may be, namely the flexbox styles), it may make sense for this option to be a prop on the EUI side. If I'm wrong though, and there are little to no style implications, then I suppose there's no harm in having the Shared UX team implement that restriction.

@cee-chen
Copy link
Contributor

@ryankeairns Quick reminder to move out the requested EuiBreadcrumbs behavior out of this issue and into a separate one - as of the next EuiCollapsibleNavBeta PR, my work on the collapsible nav itself will be complete and I'll be closing this issue.

@cee-chen
Copy link
Contributor

cee-chen commented Aug 4, 2023

This issue will close once #7034 merges, as a wholly usable MVP will be available for Kibana at that point. As a heads up, here are several things we skipped implementing as a conscious choice:

  1. Width animations on desktop:
    When in overlay/mobile mode, EuiFlyouts have a nice & performant transform animation that makes them transition out from the left or right. When in push/desktop mode, EuiFlyout (and therefore EuiCollapsibleNavBeta) does not have this animation.
    While it may be worth investigating trying to find a way to do this in the future, it will need to be heavily performance-tested as adding an animation to the push flyout will cause the entire page and its content to repaint with multiple layout flow changes.

  2. Docked popover sub-sub-items:

    Currently, nested items render as accordions. There is an argument to be made here for using EuiContextMenu instead; however this would require a significant amount of dev work to transmute EuiCollapsibleNavItem's data structure to EuiContextMenu's data structure. For now, we've decided not to take this approach, although this can certainly be revisited in the future if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants