[Feature] Added `EuiCollapsibleNav` component by cchaos · Pull Request #2977 · elastic/eui

cchaos · 2020-03-03T23:47:33Z

Feature branch

This is the primary container for the new EuiCollapsibleNav component. It doesn't do much but

extend EuiFlyout,
push to the left side,
add docked ability, and
setup some responsive behavior

Basic flyout behavior

Responsive behavior with docking

Checklist

Check against all themes for compatibility in both light and dark modes
Checked in mobile
Checked in IE11 and Firefox
Props have proper autodocs
Added documentation examples
Added or updated jest tests
~~[ ] Checked for breaking changes and labeled appropriately~~
Checked for accessibility including keyboard-only and screenreader modes
~~[ ] A changelog entry exists and is marked appropriately~~ Will be added via the Feature Branch

cchaos · 2020-03-03T23:49:52Z

@myasonik What kind of aria-alert or whatever do you think we will need for when the user initiates the opening of the navigation drawer?

@thompsongl Is there any way to add tests for the responsive behavior? Can it mock setting the window to a certain size?

kibanamachine · 2020-03-04T00:05:30Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_2977/

snide

Some smaller stuff I noticed. Couple questions.

src-docs/src/views/collapsible_nav/collapsible_nav_example.js

src/components/collapsible_nav/_collapsible_nav.scss

src/components/collapsible_nav/collapsible_nav.tsx

src-docs/src/views/collapsible_nav/collapsible_nav_example.js

src/components/collapsible_nav/_collapsible_nav.scss

myasonik

When undocking the nav, I was surprised when the nav was still open. What do you think about closing the nav when undocked?

src/components/collapsible_nav/collapsible_nav.tsx

cchaos · 2020-03-04T21:19:47Z

When undocking the nav, I was surprised when the nav was still open. What do you think about closing the nav when undocked?

The current behavior is how it was working during the Usability tests. We didn't hear any feedback specifically on this interaction so I'm not inclined to change it.

cchaos · 2020-03-04T21:26:48Z

I should also mention that the example for CollapsibleNav is not complete. It will get fleshed out with all the other inner content, but for now I'm just trying to nail the actual "drawer" part.

thompsongl · 2020-03-04T23:10:15Z

Is there any way to add tests for the responsive behavior? Can it mock setting the window to a certain size?

I'm not sure that any @media queries would get used (jsdom does not actually draw anything), but render changes based on window.innerwidth in JS should get picked with something like

https://gist.github.com/thompsongl/cec1e5be12078ff9e56dc78296b88e27

Also added a `close` button

cchaos · 2020-03-05T21:57:50Z

Ok the component has been updated to not render a EuiFlyout, but just reuses some styles and copies some functionality over. I addressed (most) of the other comments as well.

I also added a visable "close" button (though I supposed we could make it a accessibleOnFocus element instead).

kibanamachine · 2020-03-05T22:15:03Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_2977/

myasonik · 2020-03-09T15:13:07Z

Is there any way to have the portal render at the start of the document for when rendering the docked version of the nav? Having it at the end of the document, I think, is pretty confusing from a tab order perspective.

snide

My issues were addressed and I did a quick code review of the new restructuring. I'd suggest like DataGrid, setting up a PR for the feature branch merge (in draft mode) and then keeping links to these sub PRs in that PR. That way you'll have a nice reference when you need to write your CL.

We don't often use extends, but looks OK here. Mixing might be better so you can pass stuff to it eventually. Very small comment though.

cchaos · 2020-03-09T19:27:39Z

Is there any way to have the portal render at the start of the document for when rendering the docked version of the nav? Having it at the end of the document, I think, is pretty confusing from a tab order perspective.

The component does not live in a portal. It's DOM will get inserted wherever you place the component. The example in the EUI docs is at the tail end of the DOM structure so that's where it gets rendered.

cchaos · 2020-03-09T19:29:22Z

I'd suggest setting up a PR for the feature branch merge (in draft mode)

Yep, I have to have new commits in the branch before Github allows me to create a PR, so I need this one in first.

thompsongl

LGTM; works as expected

* Setting up file structure * Added EuiFlyout to render, moved to left, and added docking * mock euioverlaymask * Better docs for EuiCollapsibleNav * Cleanup css * Adding responsive behavior * No longer using EuiFlyout directly * added a `close` button

* [Feature] Added `EuiCollapsibleNav` component (#2977) * [New Nav Feature] Added `ghost` colored EuiListGroupItem (#3018) * [New Nav Feature] Created `EuiCollapsibleGroup` (#3031) * [New Nav Feature] EuiPinnableListGroup (#3061) * [K8 Nav Feature] Added `home` and `menu` glyphs to EuiIcon (#3109) * [New Nav Feature] Final docs examples and patterns (#3117) * [New Nav Feature] Move collapsible nav toggle button to be part of EuiCollapsibleNav (#3168)

* [Feature] Added `EuiCollapsibleNav` component (elastic#2977) * [New Nav Feature] Added `ghost` colored EuiListGroupItem (elastic#3018) * [New Nav Feature] Created `EuiCollapsibleGroup` (elastic#3031) * [New Nav Feature] EuiPinnableListGroup (elastic#3061) * [K8 Nav Feature] Added `home` and `menu` glyphs to EuiIcon (elastic#3109) * [New Nav Feature] Final docs examples and patterns (elastic#3117) * [New Nav Feature] Move collapsible nav toggle button to be part of EuiCollapsibleNav (elastic#3168)

cchaos and others added 6 commits March 3, 2020 18:07

Setting up file structure

52fc52f

Added EuiFlyout to render, moved to left, and added docking

681856f

mock euioverlaymask

ea603f9

Better docs for EuiCollapsibleNav

e680fa2

Cleanup css

0f4fd62

Adding responsive behavior

8e51c02

cchaos requested review from elizabetdev and thompsongl March 4, 2020 01:20

snide reviewed Mar 4, 2020

View reviewed changes

myasonik suggested changes Mar 4, 2020

View reviewed changes

No longer using EuiFlyout directly

8574f47

Also added a `close` button

cchaos requested review from myasonik and snide March 5, 2020 21:57

snide approved these changes Mar 9, 2020

View reviewed changes

myasonik approved these changes Mar 9, 2020

View reviewed changes

thompsongl approved these changes Mar 9, 2020

View reviewed changes

cchaos changed the title ~~Added EuiCollapsibleNav component~~ [Feature] Added EuiCollapsibleNav component Mar 9, 2020

cchaos merged commit 7d7df65 into elastic:feature/collapsible_nav Mar 9, 2020

cchaos deleted the into-feature/collapsible_nav/nav branch March 9, 2020 20:19

cchaos mentioned this pull request Mar 9, 2020

[Feature Branch] EuiCollapsibleNav #3019

Merged

8 tasks

myasonik mentioned this pull request Mar 10, 2020

Add a floating close button to all "popovers" #3024

Closed

snide mentioned this pull request May 15, 2020

[Meta] Deprecation and Beta schedule #1469

Open

41 tasks

cchaos mentioned this pull request Sep 23, 2020

Finish Dec-2019 & July-2020 deprecations #4065

Merged

7 tasks

Conversation

cchaos commented Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Feature branch

Basic flyout behavior

Responsive behavior with docking

Checklist

Uh oh!

cchaos commented Mar 3, 2020

Uh oh!

kibanamachine commented Mar 4, 2020

Uh oh!

snide left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

myasonik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cchaos commented Mar 4, 2020

Uh oh!

cchaos commented Mar 4, 2020

Uh oh!

thompsongl commented Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cchaos commented Mar 5, 2020

Uh oh!

kibanamachine commented Mar 5, 2020

Uh oh!

myasonik commented Mar 9, 2020

Uh oh!

snide left a comment

Choose a reason for hiding this comment

Uh oh!

cchaos commented Mar 9, 2020

Uh oh!

cchaos commented Mar 9, 2020

Uh oh!

thompsongl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cchaos commented Mar 3, 2020 •

edited

Loading

thompsongl commented Mar 4, 2020 •

edited

Loading