[EuiHeaderLinks] Providing more flexibility in display #4046

cchaos · 2020-09-16T23:23:12Z

And update a lot of the Header examples.

This may look like a lot of changes, but it's because I fixed up most of the Header page examples to:

No longer import external doc components (like HeaderUserMenu and HeaderSpacesMenu). This would cause the CodeSandbox examples to break and really we only need to display them once (or twice) and the rest of the examples just show the button.
All Elastic logos instead of Kibana
Remove all actual links that might send the user to the index page. Basically getting ahead of some of the # fixes.

EuiHeaderLinks

Added gutterSize to adjust the spacing between direct children. And extended some props on the "mobile" version with popoverBreakpoints, popoverButtonProps, and popoverProps. And then I used the EuiHideFor and EuiShowFor to render/not render the inline links versus the popover version depending on the popoverBreakpoints
instead of using CSS media queries which are hard to adjust at the consumer level.

EuiHide/ShowFor adjustment

As an extra ease of use for the EuiHideFor and EuiShowFor components, I'm allowing the sizes prop also accept the strings all and none.

Checklist

Check against all themes for compatibility in both light and dark modes
Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs
Added documentation
Checked Code Sandbox works for the any docs 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

- No importing of external doc examples - All Elastic logos - Remove all actual links that might send the user to the index page - Only the first example shows all the popovers - The Full screen demo has the rest

Then update `popoverBreakpoints` prop in EuiHeaderLinks to also allow for `all` and `none`.

cchaos · 2020-09-16T23:27:08Z

src-docs/src/views/header/header.js

-import HeaderAppMenu from './header_app_menu';
-import HeaderUserMenu from './header_user_menu';
-import HeaderSpacesMenu from './header_spaces_menu';


All these imports to to be directly declared in this file. Now the CodeSandbox works 🎉

src-docs/src/views/header/header.js

cchaos · 2020-09-16T23:29:02Z

src-docs/src/views/header/header_alert.js

-
-import HeaderUserMenu from './header_user_menu';
-import HeaderSpacesMenu from './header_spaces_menu';
-import HeaderUpdates from './header_updates';


Again moved the HeaderUpdates component directly in this file. It still needs an actual example all on its own, but that will take time.

src/components/popover/popover.tsx

kibanamachine · 2020-09-17T00:17:32Z

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

elizabetdev

It looks good! 🎉

I looked at the code and tested the docs and sandboxes. I just added a suggestion to replace the <Link> from react-router-dom.

src-docs/src/views/header/header.js

kibanamachine · 2020-09-17T20:22:21Z

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

chandlerprall

One small change requested.

Also, I'm curious what the use case for all/none in show/hide is? Seems like it would be easier to not include the content at all (when wanting to never show or always hide) or not wrap with a show/hide tag when it should always be included.

src/components/header/header_links/header_links.tsx

cchaos · 2020-09-18T19:54:42Z

Also, I'm curious what the use case for all/none in show/hide is? Seems like it would be easier to not include the content at all (when wanting to never show or always hide) or not wrap with a show/hide tag when it should always be included.

I agree that it seems odd to allow such a value, but I found that it makes it much easier (on the EUI side) to support the inclusion or exclusion of responsive breakpoints entirely. So in EuiHeaderLinks, I can allow customization of when to show/hide the popover version via:

eui/src/components/header/header_links/header_links.tsx

Line 53 in 1553d33

popoverBreakpoints?: EuiBreakpointSize[] | 'all' | 'none';

And then be able to pass that exact value to the EuiHide/ShowFor components without needing to do some extra calculation/logic to handle those cases.

It's more of an ease of use for us internally than it might necessarily be for consumers, but it doesn't hurt to provide it at that level instead of hiding behind a service.

chandlerprall

Thanks for the usage info for show/hide breakpoints - figured it was something like that, wanted to double check. Everything LGTM!

kibanamachine · 2020-09-18T20:27:47Z

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

elizabetdev

LGTM! 🎉

kibanamachine · 2020-09-22T19:57:03Z

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

cchaos requested review from chandlerprall and elizabetdev September 16, 2020 23:23

cchaos added 8 commits September 16, 2020 19:24

[EuiHeaderLinks] Adding gutterSize flexibility

622cf5d

Overflow?

ebfc407

[EuiHeaderLinks] Fix spaing and extend popover props

f4dc3a0

Removing # from hrefs in examples

3f15295

[EuiPopover] Fix container from being passed to DOM as attribute

b8f1b9a

Fixing up all the header examples

f62b4e0

- No importing of external doc examples - All Elastic logos - Remove all actual links that might send the user to the index page - Only the first example shows all the popovers - The Full screen demo has the rest

[EuiHide/ShowFor] Add all and none options for sizes

7347713

Then update `popoverBreakpoints` prop in EuiHeaderLinks to also allow for `all` and `none`.

Update docs for hide/showFor

e74cc24

cchaos force-pushed the flexing/header_links branch from cf00811 to e74cc24 Compare September 16, 2020 23:24

Clean up and CL

f4048b5

cchaos commented Sep 16, 2020

View reviewed changes

elizabetdev suggested changes Sep 17, 2020

View reviewed changes

src-docs/src/views/header/header.js Show resolved Hide resolved

cchaos and others added 2 commits September 17, 2020 15:54

Fixing all popover scrolls in fixed examples

c4f6e58

Merge branch 'master' into flexing/header_links

1553d33

cchaos requested a review from elizabetdev September 18, 2020 14:38

chandlerprall mentioned this pull request Sep 18, 2020

Integrate EuiLink into the docs' routing #4052

Closed

chandlerprall suggested changes Sep 18, 2020

View reviewed changes

src/components/header/header_links/header_links.tsx Outdated Show resolved Hide resolved

Fix mouse event TS

7409d9f

Merge branch 'master' into flexing/header_links

007a88d

cchaos requested a review from chandlerprall September 18, 2020 19:55

chandlerprall approved these changes Sep 18, 2020

View reviewed changes

Typo

e4e12db

elizabetdev approved these changes Sep 21, 2020

View reviewed changes

Merge branch 'master' into flexing/header_links

41073a0

cchaos merged commit b6f793d into elastic:master Sep 22, 2020

cchaos deleted the flexing/header_links branch September 22, 2020 20:16

chandlerprall mentioned this pull request Sep 29, 2020

Upgraded EUI to v29.3.0 elastic/kibana#78870

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiHeaderLinks] Providing more flexibility in display #4046

[EuiHeaderLinks] Providing more flexibility in display #4046

cchaos commented Sep 16, 2020 •

edited

Loading

cchaos Sep 16, 2020

cchaos Sep 16, 2020

kibanamachine commented Sep 17, 2020

elizabetdev left a comment

kibanamachine commented Sep 17, 2020

chandlerprall left a comment

cchaos commented Sep 18, 2020

chandlerprall left a comment

kibanamachine commented Sep 18, 2020

elizabetdev left a comment

kibanamachine commented Sep 22, 2020

[EuiHeaderLinks] Providing more flexibility in display #4046

[EuiHeaderLinks] Providing more flexibility in display #4046

Conversation

cchaos commented Sep 16, 2020 • edited Loading

And update a lot of the Header examples.

EuiHeaderLinks

EuiHide/ShowFor adjustment

Checklist

cchaos Sep 16, 2020

Choose a reason for hiding this comment

cchaos Sep 16, 2020

Choose a reason for hiding this comment

kibanamachine commented Sep 17, 2020

elizabetdev left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 17, 2020

chandlerprall left a comment

Choose a reason for hiding this comment

cchaos commented Sep 18, 2020

chandlerprall left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 18, 2020

elizabetdev left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 22, 2020

cchaos commented Sep 16, 2020 •

edited

Loading