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

[Canvas] Top Menu #59982

Merged
merged 40 commits into from
Apr 23, 2020
Merged

[Canvas] Top Menu #59982

merged 40 commits into from
Apr 23, 2020

Conversation

cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Mar 12, 2020

Summary

Related to #35873.
Closes #62927.

This redesigns the layout of the WorkpadHeader component and replaces the various icon buttons up top with an easier to understand menu layout.

Mar-31-2020 13-28-34

Add Element Menu

Screen Shot 2020-04-15 at 9 59 07 AM

Chart elements

Screen Shot 2020-04-15 at 9 59 15 AM

Image elements

Screen Shot 2020-04-15 at 9 59 22 AM

Filter elements

Screen Shot 2020-04-15 at 9 59 27 AM

Progress elements

Screen Shot 2020-04-15 at 9 59 34 AM

Other elements

Screen Shot 2020-04-15 at 9 59 40 AM

View menu

Screen Shot 2020-04-15 at 9 59 51 AM

Share menu

Screen Shot 2020-04-15 at 10 00 03 AM

Options menu (refresh and autoplay controls)

Note: I'm planning on breaking up these two settings into separate options under the View menu in a future PR.

Screen Shot 2020-04-15 at 10 00 13 AM

Changes:

  • adds view_menu component
  • removes donut and tilted pie elements
  • removes tags previously used for elements (i.e. filter, graphic, text, etc)
  • replaces tags with type property on element spec
  • renames workpad_export to share_menu
  • moves element list from element_types into new element_menu component
  • renames element_types to saved_elements_modal
  • fixes file picker size in asset_manager
  • removes preview images for all elements
  • adds a filter debug element
  • updates custom element functional tests

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cqliu1 cqliu1 added the WIP Work in progress label Mar 12, 2020
@ryankeairns
Copy link
Contributor

I've only just looked at the screenshot in the description, but...

tenor-933097

@ryankeairns ryankeairns mentioned this pull request Mar 20, 2020
7 tasks
@cqliu1 cqliu1 force-pushed the canvas/top-menu branch 2 times, most recently from 5db3841 to f617f55 Compare March 24, 2020 17:12
@ryankeairns
Copy link
Contributor

Created a design PR with a few minor tweaks to better align with other Kibana top menus:

PR4U 👉 cqliu1#11

@cqliu1 cqliu1 added impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:large Large Level of Effort Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Apr 2, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas (Team:Canvas)

@ryankeairns
Copy link
Contributor

You can do it!

@cqliu1 cqliu1 marked this pull request as ready for review April 16, 2020 16:29
@cqliu1 cqliu1 requested review from a team as code owners April 16, 2020 16:29
@cqliu1 cqliu1 added review v7.8.0 v8.0.0 and removed WIP Work in progress labels Apr 16, 2020
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

We'd iterated on the design during the making of this PR. Nothing more to add - it looks and works great. Really excited to hear user feedback!

Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm -- this is great!

"xpack.canvas.workpadHeaderViewMenu.zoomOutText": "ズームアウト",
"xpack.canvas.workpadHeaderViewMenu.zoomPanelTitle": "ズーム:",
"xpack.canvas.workpadHeaderViewMenu.zoomPrecentageValue": "リセット",
"xpack.canvas.workpadHeaderViewMenu.zoomResetText": "{scalePercentage}%",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll likely wanna still check with the i18n team to make sure all the i18n changes are a-ok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! I checked in with the localization team on Slack, and they said these changes were fine.

import { euiPaletteColorBlind } from '@elastic/eui';
const euiVisPalette = euiPaletteColorBlind();

export const proportion = () => ({ name: 'proportion', color: euiVisPalette[3] });
Copy link
Contributor

@poffdeluxe poffdeluxe Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what the hell github lol. Somehow it thinks you renamed the proportion tag js file to be uiMetric.js

Copy link
Contributor Author

@cqliu1 cqliu1 Apr 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah idk this one I don't get at all...

@spalger
Copy link
Contributor

spalger commented Apr 23, 2020

@elasticmachine merge upstream (sorry, I applied a vault update that will cause the previous build to fail)

@cqliu1 cqliu1 merged commit 1a0988f into elastic:master Apr 23, 2020
@cqliu1 cqliu1 deleted the canvas/top-menu branch April 23, 2020 04:06
@spalger
Copy link
Contributor

spalger commented Apr 23, 2020

@cqliu1 would you be able to backport this to 7.7 after it is released? I needed to backport the test skipping to 7.7.

@cqliu1
Copy link
Contributor Author

cqliu1 commented Apr 24, 2020

@spalger This PR includes a lot of UI changes that we don't want to ship in 7.7. Should I reopen the flaky test issue?

@spalger
Copy link
Contributor

spalger commented Apr 24, 2020

I definitely think we should fix the flaky test in 7.7, yeah

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 27, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@cqliu1 cqliu1 restored the canvas/top-menu branch April 27, 2020 17:59
@cqliu1 cqliu1 deleted the canvas/top-menu branch April 27, 2020 18:39
cqliu1 added a commit that referenced this pull request Apr 27, 2020
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 27, 2020
@cqliu1 cqliu1 mentioned this pull request Apr 30, 2020
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:large Large Level of Effort release_note:enhancement review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.8.0 v8.0.0
Projects
None yet
6 participants