Updates to K7 nav drawer and EUI to 9.4.0 #32864

ryankeairns · 2019-03-10T22:20:08Z

Summary

Updates Kibana to work with the latest version of the EUINavDrawer component which incorporates feedback from K7 users and reduces the implementation surface area. Changes include:

removes hover-to-expand interaction
only one menu is open at a time - either the main menu or the flyout sub menu
new fixed bottom for toggling open the side nav
nav is collapsed by default with new tooltips that appear on hover to see app name
menus are closed by clicking outside
removes pre-animation delays; speeds up the few remaining animations
menu and flyout now scroll separately

To-do

i18n for 'Recently viewed' button label and flyout title (tried using EuiI18n to no avail)
change data-test-subj names (see feedback below)
fix tests in app_menu.ts. They previously relied upon an expanded:collapsed flag that is now baked into EuiNavDrawer. I'm adding a data-test-subj to the EuiNavDrawer expand button.
build errors (mostly related to use of a ref; this is needed to reference the toggleOpen function that lives in EuiNavDrawer)

Preview

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-03-10T22:34:27Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cchaos

So nice to see cleanup inside Kibana 😄

I had a couple suggestions around the code before I take a look in the browser.

src/legacy/ui/public/chrome/directives/header_global_nav/components/header.tsx

elasticmachine · 2019-03-11T14:46:06Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-11T17:12:49Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-11T19:58:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-11T22:56:46Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-12T02:05:04Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cchaos · 2019-03-12T02:18:00Z

Can you update from master? The new EUI version is causing a SASS compile issue that was fixed in Kibana when 9.2.1 went it.

cchaos · 2019-03-12T03:05:01Z

Pushed a quick fix in case recent item names are very long so that this:

now renders as:

elasticmachine · 2019-03-12T03:59:55Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

- Removing aria-label if the same as label since the `EuiNavDrawerGroup` takes care of that - Truncating any recent items to just 64 characters and applying the `title` attribute to display the whole label.

Also comments out some logos that don’t exist that were throwing console errors

snide

Tested for functionality. Works as I'd expect. Saw one small typo.

src/legacy/ui/public/chrome/directives/header_global_nav/components/header.tsx

elasticmachine · 2019-03-12T21:38:20Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

Also comments out some logos that don’t exist that were throwing console errors

into rk/k7-nav-update

elasticmachine · 2019-03-13T22:46:42Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

Updated to EUI 9.4.0

elasticmachine · 2019-03-14T00:41:26Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

into rk/k7-nav-update

elasticmachine · 2019-03-14T15:54:36Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…ontinuing

elasticmachine · 2019-03-14T17:31:31Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* update for new eui nav drawer * remove extraAction on flyout * fix mobile menu toggle * add i18n for recently reviewed nav link * progress on updating tests * replace EuiListGroup with EuiNavDrawerGroup * Quick label fixes - Removing aria-label if the same as label since the `EuiNavDrawerGroup` takes care of that - Truncating any recent items to just 64 characters and applying the `title` attribute to display the whole label. * Adding a truncation buffer * fix navDrawerRef * Update to EUI 9.3.0 Also comments out some logos that don’t exist that were throwing console errors * remove unused import * typo fix * One DTS too many in test * …all the small things… * Upgrade plugin dependencies and fix nav drawer test * update for new eui nav drawer * remove extraAction on flyout * fix mobile menu toggle * add i18n for recently reviewed nav link * progress on updating tests * replace EuiListGroup with EuiNavDrawerGroup * Quick label fixes - Removing aria-label if the same as label since the `EuiNavDrawerGroup` takes care of that - Truncating any recent items to just 64 characters and applying the `title` attribute to display the whole label. * Adding a truncation buffer * fix navDrawerRef * Update to EUI 9.3.0 Also comments out some logos that don’t exist that were throwing console errors * remove unused import * typo fix * One DTS too many in test * …all the small things… * Upgrade plugin dependencies and fix nav drawer test * Add support for normal image paths Updated to EUI 9.4.0 * update testing appMenu.clickLink selector * leave appsMenu closed while reading and clicking links * use saveVisualizationExpectSuccess to ensure modal is closed before continuing # Conflicts: # yarn.lock

* update for new eui nav drawer * remove extraAction on flyout * fix mobile menu toggle * add i18n for recently reviewed nav link * progress on updating tests * replace EuiListGroup with EuiNavDrawerGroup * Quick label fixes - Removing aria-label if the same as label since the `EuiNavDrawerGroup` takes care of that - Truncating any recent items to just 64 characters and applying the `title` attribute to display the whole label. * Adding a truncation buffer * fix navDrawerRef * Update to EUI 9.3.0 Also comments out some logos that don’t exist that were throwing console errors * remove unused import * typo fix * One DTS too many in test * …all the small things… * Upgrade plugin dependencies and fix nav drawer test * update for new eui nav drawer * remove extraAction on flyout * fix mobile menu toggle * add i18n for recently reviewed nav link * progress on updating tests * replace EuiListGroup with EuiNavDrawerGroup * Quick label fixes - Removing aria-label if the same as label since the `EuiNavDrawerGroup` takes care of that - Truncating any recent items to just 64 characters and applying the `title` attribute to display the whole label. * Adding a truncation buffer * fix navDrawerRef * Update to EUI 9.3.0 Also comments out some logos that don’t exist that were throwing console errors * remove unused import * typo fix * One DTS too many in test * …all the small things… * Upgrade plugin dependencies and fix nav drawer test * Add support for normal image paths Updated to EUI 9.4.0 * update testing appMenu.clickLink selector * leave appsMenu closed while reading and clicking links * use saveVisualizationExpectSuccess to ensure modal is closed before continuing

Adds EUI bigfixes and backports the nav changes and dark mode changes. And includes backports for #32864 and #32728.

ryankeairns · 2019-03-16T15:38:00Z

Thanks @cchaos and @thompsongl for seeing this through!

ryankeairns requested review from snide and cchaos March 10, 2019 22:20

cchaos suggested changes Mar 11, 2019

View reviewed changes

ryankeairns added v7.0.0 v7.2.0 labels Mar 11, 2019

ryankeairns marked this pull request as ready for review March 11, 2019 17:51

ryankeairns force-pushed the rk/k7-nav-update branch from 6e1259b to 284bded Compare March 11, 2019 22:37

ryankeairns and others added 10 commits March 12, 2019 16:26

update for new eui nav drawer

60a85df

remove extraAction on flyout

71f572e

fix mobile menu toggle

e87c246

add i18n for recently reviewed nav link

b627a88

progress on updating tests

38c3068

replace EuiListGroup with EuiNavDrawerGroup

ef2ff1e

Quick label fixes

c0f5c5b

- Removing aria-label if the same as label since the `EuiNavDrawerGroup` takes care of that - Truncating any recent items to just 64 characters and applying the `title` attribute to display the whole label.

Adding a truncation buffer

f624f36

fix navDrawerRef

943807e

Update to EUI 9.3.0

390dcaa

Also comments out some logos that don’t exist that were throwing console errors

cchaos force-pushed the rk/k7-nav-update branch from 09df6f6 to 390dcaa Compare March 12, 2019 20:47

cchaos changed the title ~~Updates to K7 nav drawer~~ Updates to K7 nav drawer and EUI to 9.3.0 Mar 12, 2019

snide approved these changes Mar 12, 2019

View reviewed changes

src/legacy/ui/public/chrome/directives/header_global_nav/components/header.tsx Outdated Show resolved Hide resolved

cchaos added 6 commits March 13, 2019 17:58

Update to EUI 9.3.0

fafbbe5

Also comments out some logos that don’t exist that were throwing console errors

remove unused import

7037848

typo fix

d41b062

One DTS too many in test

0887994

…all the small things…

19ce0e1

Upgrade plugin dependencies and fix nav drawer test

8fcdd66

cchaos force-pushed the rk/k7-nav-update branch from ea976f4 to 8fcdd66 Compare March 13, 2019 21:58

Merge branch 'rk/k7-nav-update' of https://github.com/ryankeairns/kibana

530e8a1

into rk/k7-nav-update

Add support for normal image paths

af828e0

Updated to EUI 9.4.0

cchaos changed the title ~~Updates to K7 nav drawer and EUI to 9.3.0~~ Updates to K7 nav drawer and EUI to 9.4.0 Mar 13, 2019

thompsongl added 2 commits March 14, 2019 08:29

Merge branch 'rk/k7-nav-update' of https://github.com/ryankeairns/kibana

893807d

into rk/k7-nav-update

update testing appMenu.clickLink selector

d717eee

cchaos self-requested a review March 14, 2019 15:27

cchaos approved these changes Mar 14, 2019

View reviewed changes

thompsongl added 2 commits March 14, 2019 11:40

leave appsMenu closed while reading and clicking links

5ec67c9

use saveVisualizationExpectSuccess to ensure modal is closed before c…

821b362

…ontinuing

cchaos merged commit e9885e2 into elastic:master Mar 14, 2019

cchaos added the release_note:skip Skip the PR/issue when compiling release notes label Mar 14, 2019

cchaos mentioned this pull request Mar 14, 2019

[7.x] Updates to K7 nav drawer and EUI to 9.4.0 (#32864) #33260

Merged

cchaos added a commit that referenced this pull request Mar 14, 2019

Updates to K7 nav drawer and EUI to 9.4.0 (#32864) (#33260)

5f8e0fc

snide mentioned this pull request Mar 15, 2019

Eui/6.10.5 in Kibana 7.0 - A design feedback PR before GA #33273

Merged

7 tasks

snide added a commit that referenced this pull request Mar 15, 2019

Eui/6.10.5 in Kibana 7.0 - A design feedback PR before GA (#33273)

ddc0187

Adds EUI bigfixes and backports the nav changes and dark mode changes. And includes backports for #32864 and #32728.

ryankeairns mentioned this pull request Mar 25, 2019

Navigation hides the management sub menu #30331

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates to K7 nav drawer and EUI to 9.4.0 #32864

Updates to K7 nav drawer and EUI to 9.4.0 #32864

ryankeairns commented Mar 10, 2019 •

edited by cchaos

Loading

elasticmachine commented Mar 10, 2019

cchaos left a comment

elasticmachine commented Mar 11, 2019

elasticmachine commented Mar 11, 2019

elasticmachine commented Mar 11, 2019

elasticmachine commented Mar 11, 2019

elasticmachine commented Mar 12, 2019

cchaos commented Mar 12, 2019

cchaos commented Mar 12, 2019

elasticmachine commented Mar 12, 2019

snide left a comment

elasticmachine commented Mar 12, 2019

elasticmachine commented Mar 13, 2019

elasticmachine commented Mar 14, 2019

elasticmachine commented Mar 14, 2019

elasticmachine commented Mar 14, 2019

ryankeairns commented Mar 16, 2019

Updates to K7 nav drawer and EUI to 9.4.0 #32864

Updates to K7 nav drawer and EUI to 9.4.0 #32864

Conversation

ryankeairns commented Mar 10, 2019 • edited by cchaos Loading

Summary

To-do

Preview

Checklist

For maintainers

elasticmachine commented Mar 10, 2019

💔 Build Failed

cchaos left a comment

Choose a reason for hiding this comment

elasticmachine commented Mar 11, 2019

💔 Build Failed

elasticmachine commented Mar 11, 2019

💔 Build Failed

elasticmachine commented Mar 11, 2019

💔 Build Failed

elasticmachine commented Mar 11, 2019

💔 Build Failed

elasticmachine commented Mar 12, 2019

💔 Build Failed

cchaos commented Mar 12, 2019

cchaos commented Mar 12, 2019

elasticmachine commented Mar 12, 2019

💔 Build Failed

snide left a comment

Choose a reason for hiding this comment

elasticmachine commented Mar 12, 2019

💔 Build Failed

elasticmachine commented Mar 13, 2019

💔 Build Failed

elasticmachine commented Mar 14, 2019

💔 Build Failed

elasticmachine commented Mar 14, 2019

💔 Build Failed

elasticmachine commented Mar 14, 2019

💚 Build Succeeded

ryankeairns commented Mar 16, 2019

ryankeairns commented Mar 10, 2019 •

edited by cchaos

Loading