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

Align top menus to new header design #78332

Closed

Conversation

ryankeairns
Copy link
Contributor

@ryankeairns ryankeairns commented Sep 23, 2020

Relates to #72331 (comment)

Summary

➡️ There will likely be opinions on this one, so I'm not pretending that this is a slam dunk of a PR... as evidenced by all the screenshots. Please take a look, try this branch locally, try master again, and then see what you think.

With the new stacked header design, 'top menus' (in the legacy Kibana apps) were moved from the app level to the chrome/header level. For some (author included :) ), this has resulted in an undesirable visual design/UX outcome for apps that used filled buttons in their menu. For example:

Competing buttons in Dashboard

Screen Shot 2020-09-23 at 12 59 14 PM

Competing buttons in Lens

Screen Shot 2020-09-23 at 12 59 02 PM

Proposed 7.10 solution introduced by this PR

  • Replace the filled (emphasized) buttons with an empty + icon button and make it the first item.
  • For Dashboard, make 'Edit' the first item and add an icon even though it was previously not emphasized.
  • Where possible, use a filled-style icon for additional emphasis (e.g. checkInCircleFilled and plusInCircleFilled)
  • Apps that did not have an emphasized button (Discover, Graph) remain unchanged.

Affected apps (Dashboard, Lens, Visualize, Maps)

Dashboard (edit mode)

Screen Shot 2020-09-23 at 1 14 42 PM

Dashboard (view mode)

Screen Shot 2020-09-23 at 1 18 35 PM

Lens

Screen Shot 2020-09-23 at 1 16 15 PM

Visualize

Screen Shot 2020-09-23 at 1 15 57 PM

Visualize (from Dashboard)

Screen Shot 2020-09-23 at 2 46 55 PM

Maps

Screen Shot 2020-09-23 at 1 16 34 PM

Alternative solutions explored for 7.10

  1. Convert filled/primary buttons to empty buttons with icons
    Proposed solution in previous section ; single icon on first item provides differentiation while avoiding competing buttons

Screen Shot 2020-09-23 at 1 14 42 PM

  1. Convert all buttons to empty buttons with icons
    Less desirable as all items are emphasized equally; less clear CTA

Screen Shot 2020-09-22 at 3 49 06 PM

  1. Leave menu buttons as-is, but change the Super Date Picker button style
    Doesn't seem to address the original issue of competing buttons; less desirable visually as well

Screen Shot 2020-09-22 at 5 16 25 PM

Looking ahead

In addition to the top menu, teams may begin exploring additional UI enhancements if they desire to further emphasize core actions. For example, the Dashboard team is exploring adding a toolbar within their UI (for this and other reasons):

Screen Shot 2020-09-04 at 10 08 40 AM


Checklist

Delete any items that are not applicable to this PR.

@ryankeairns ryankeairns force-pushed the rk/top-menu-consistency branch from b869dd5 to 334db08 Compare September 23, 2020 20:36
@ryankeairns ryankeairns force-pushed the rk/top-menu-consistency branch from 334db08 to e9883ff Compare September 24, 2020 18:56
@ryankeairns ryankeairns marked this pull request as ready for review September 28, 2020 21:25
@ryankeairns ryankeairns requested a review from a team September 28, 2020 21:25
@ryankeairns ryankeairns requested a review from a team as a code owner September 28, 2020 21:25
@ryankeairns ryankeairns added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 labels Sep 28, 2020
@ryankeairns ryankeairns requested review from a team September 28, 2020 21:26
@ryankeairns
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

kibanamachine commented Oct 1, 2020

💔 Build Failed

Failed CI Steps

Metrics [docs]

async chunks size

id before after diff
lens 1010.4KB 1010.5KB +15.0B
maps 3.3MB 3.3MB -19.0B
visualize 271.7KB 271.7KB +47.0B
total +43.0B

page load bundle size

id before after diff
dashboard 570.4KB 570.4KB +9.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@ryankeairns
Copy link
Contributor Author

Had a chat with Caroline and Dave, going to try one more thing here - using hollow style (bordered) buttons for the primary CTAs. That added benefit is that these will shift to a light fill in the Amsterdam theme which is targeted to be the default theme later in 7.x.

I'll make those adjustments and update this PR with screenshots.

@ryankeairns
Copy link
Contributor Author

Closing in favor of #79206

@ryankeairns ryankeairns closed this Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants