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

Context menu #76497

Merged
merged 53 commits into from
Sep 21, 2020
Merged

Context menu #76497

merged 53 commits into from
Sep 21, 2020

Conversation

streamich
Copy link
Contributor

@streamich streamich commented Sep 2, 2020

Summary

Closes #66051

View mode:

image

Edit mode:

image

When user clicks "More":

image

In this PR the separator line is removed and will be added once it is available in EUI, see: elastic/eui#4018

We discussed this and agreed that it is ok to merge without the separator line, if it is not yet available in EUI.

I've added some examples to UI Actions example plugin, to show how the menu will look like with various actions:

yarn start --run-examples

image

Checklist

For maintainers

@streamich
Copy link
Contributor Author

@elasticmachine merge upstream

@streamich streamich requested a review from Dosant September 16, 2020 11:41
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks for addressing feedback

Copy link
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

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

Overall, looks good. Just a remark: when we switch to by value embeddables, Add to library action will need to live on the main context menu, not under More:
Screenshot 2020-09-18 at 13 37 52

Screenshot 2020-09-18 at 13 38 02

Some other comments/questions below 👇

@streamich
Copy link
Contributor Author

streamich commented Sep 18, 2020

... when we switch to by value embeddables, Add to library action will need to live on the main context menu, not under More ...

@majagrubic that is possible to do, you will just need to set the order on Add to library action higher. How it works now, is if there are more than 4 items, it shows only the top 3 and hides the remaining ones under the More submenu.


BTW, thanks for a speedy review.

Copy link
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

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

Thanx for addressing the comments, looks good!

@streamich
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
embeddableEnhanced 16 +1 15
uiActions 34 -136 170
total -135

page load bundle size

id value diff baseline
dashboardEnhanced 185.9KB +241.0B 185.7KB
embeddable 435.1KB -35.0B 435.1KB
embeddableEnhanced 40.4KB +317.0B 40.1KB
uiActions 223.4KB -44.6KB 268.1KB
total -44.1KB

History

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

@streamich streamich merged commit 4b49e5a into elastic:master Sep 21, 2020
streamich added a commit that referenced this pull request Sep 21, 2020
* feat: 🎸 add grouping to presentable interface

* feat: 🎸 add group to "Explore underlying" data action

* refactor: 💡 return panel list and simplify context creation

* refactor: 💡 simplify context menu builder code

* refactor: 💡 further simplify context menu builder code

* feat: 🎸 add grouping to context menu builder

* feat: 🎸 add icon to drilldowns group

* fix: 🐛 sort in the other order

* feat: 🎸 group drilldown actions in edit mode

* fix: 🐛 fix TypeScript error

* feat: 🎸 wrap long context menu list into a submenu

* feat: 🎸 improve context menu long list wrapping

* feat: 🎸 display drilldowns panel at the bottom of main panel

* feat: 🎸 add separator line for context menu

* test: 💍 add basic context menu builder unit tests

* feat: 🎸 remove meta decoratiosn from generated menu

* test: 💍 add test subject attribute to "More" menu item

* chore: 🤖 remove separator line and add comment about EUI

* test: 💍 update Jest snapshots

* chore: 🤖 revert back change of showing both drilldown options

* test: 💍 add context menu samples to example plugin

* feat: 🎸 collapse long groups into a sub-panel

* test: 💍 add context menu panel edit mode examples

* test: 💍 fix OSS functional test

* test: 💍 fix X-Pack functional tests

* fix: 🐛 re-introduce item sorting by title

* test: 💍 allow explicitly opening more menu

* test: 💍 try opening more panel in functional tests

* test: 💍 disable some tests

* chore: 🤖 remove unused code

* test: 💍 use action test helper in unit tests

* refactor: 💡 add helper utility to generate actions in examples

* test: 💍 disable one more functional test

* test: 💍 improve how inspector is opened in functional tests

* test: 💍 enable functional test

* refactor: 💡 convert test suite to typescript

* test: 💍 move panel replace tests into a separate test suite

* test: 💍 move panel cloning tests to a separate test suite

* test: 💍 set up dashboard context menu test suite

* test: 💍 enable few panel context menu tests

* test: 💍 enable saved search panel tests

* test: 💍 enable expanded panel context menu tests

* test: 💍 remove render complete awaits

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Drilldowns Embeddable panel Drilldowns Feature:Embedding Embedding content via iFrame Feature:UIActions UI actions. These are client side only, not related to the server side actions.. 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.

Update and re-organize Dashboard panel action menu
5 participants