[EuiContextMenu] Add line separator as menu item #4018

streamich · 2020-09-08T10:51:31Z

Summary

This is a Proof-of-Concept for line separator that we need for elastic/kibana#66051

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

kibanamachine · 2020-09-08T11:11:17Z

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

src/components/context_menu/context_menu_item.tsx

src/components/context_menu/context_menu.tsx

kibanamachine · 2020-09-10T13:55:54Z

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

cchaos · 2020-09-10T18:22:22Z

Jenkins, test this

kibanamachine · 2020-09-10T18:46:18Z

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

cchaos

Thanks @streamich ! I think this looks pretty good. I'm noticing that panel props aren't very helpful in our docs at the moment so we'll want to directly add a line in our docs about how to add this separator. Can you add something to the Using panels with mixed items & content section of context_menu_example.js?

Also, I'll update the checklist in the summary, the main outstanding one being a Changelog entry.

src/components/context_menu/context_menu.tsx

streamich · 2020-09-14T14:51:42Z

Thanks @cchaos for the review! I've updated the docs and added a line to CHANGELOG.

kibanamachine · 2020-09-14T15:14:17Z

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

chandlerprall · 2020-09-14T15:51:53Z

jenkins test this

kibanamachine · 2020-09-14T16:17:05Z

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

cchaos

Thanks for tackling the documentation! Just a couple suggestions on wording. But overall design and docs LGTM 👍

I'll pass it to @chandlerprall for the code side.

src-docs/src/views/context_menu/context_menu_example.js

… props

chandlerprall

One last request.

Additionally, I pushed a small refactor that is tangentially related: now that EuiHorizontalRuleProps is being exported as an externally accessible interface, this ensures it contains all of the applicable props that a usage could specify.

src/components/context_menu/context_menu.tsx

kibanamachine · 2020-09-14T17:41:35Z

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

Co-authored-by: Caroline Horn <[email protected]>

kibanamachine · 2020-09-15T09:29:30Z

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

chandlerprall

Changes LGTM! Pulled & tested types locally - feel free to merge, @streamich

streamich · 2020-09-17T16:06:26Z

@chandlerprall @cchaos Wanted to ask you, do you think this change will make Kibana 7.10, so we can actually make use of it already in 7.10?

streamich · 2020-09-17T19:03:46Z

OK, I see it was merged.

chandlerprall · 2020-09-17T20:45:31Z

Yep! We update Kibana's master branch to the new EUI release, and then backport that to the 7.x branch - this should certainly land in time to be used in 7.10

streamich mentioned this pull request Sep 8, 2020

Context menu elastic/kibana#76497

Merged

2 tasks

chandlerprall suggested changes Sep 8, 2020

View reviewed changes

src/components/context_menu/context_menu_item.tsx Outdated Show resolved Hide resolved

streamich added 3 commits September 10, 2020 15:27

feat: 🎸 add ability to add separator line in context menu

d2cf7c9

fix: 🐛 set buttonRef only on ContextMenuItem compnents

094f427

feat: 🎸 add separate { isSeparator: true } interface for h-rule

39f7f2b

streamich force-pushed the menu-line branch from 7d3d060 to 39f7f2b Compare September 10, 2020 13:31

chore: 🤖 delete unused prop

4fad887

streamich requested a review from chandlerprall September 10, 2020 13:36

cchaos reviewed Sep 10, 2020

View reviewed changes

src/components/context_menu/context_menu.tsx Outdated Show resolved Hide resolved

feat: 🎸 pass-through EuiHorizontalRule props

5408553

streamich requested a review from cchaos September 10, 2020 15:34

cchaos suggested changes Sep 10, 2020

View reviewed changes

src/components/context_menu/context_menu.tsx Outdated Show resolved Hide resolved

cchaos changed the title ~~PoC for menu item line separator~~ [EuiContextMenu] Add line separator as menu item Sep 10, 2020

streamich added 4 commits September 14, 2020 13:21

Merge remote-tracking branch 'origin/master' into menu-line

8ffbed8

feat: 🎸 make line have not margin by default

8d9a020

chore: 🤖 add changelog entry

c7dc470

docs: ✏️ add separaotr documentation

b2cadbc

streamich requested a review from cchaos September 14, 2020 14:51

cchaos approved these changes Sep 14, 2020

View reviewed changes

src-docs/src/views/context_menu/context_menu_example.js Outdated Show resolved Hide resolved

src-docs/src/views/context_menu/context_menu_example.js Outdated Show resolved Hide resolved

Refactored EuiHorizontalRuleProps to contain the full set of accepted…

de6cc5e

… props

chandlerprall suggested changes Sep 14, 2020

View reviewed changes

src/components/context_menu/context_menu.tsx Outdated Show resolved Hide resolved

streamich and others added 4 commits September 15, 2020 10:58

Update src-docs/src/views/context_menu/context_menu_example.js

41b0c11

Co-authored-by: Caroline Horn <[email protected]>

docs: ✏️ update docs as per review

0df852b

feat: 🎸 wrap context menu item interface in ExclusiveUnion

309c77a

chore: 🤖 merge latest master

b6ad912

streamich requested a review from chandlerprall September 15, 2020 09:10

chandlerprall approved these changes Sep 15, 2020

View reviewed changes

streamich merged commit ee78ad0 into master Sep 16, 2020

streamich deleted the menu-line branch September 16, 2020 10:53

thompsongl mentioned this pull request Sep 17, 2020

Upgrade EUI to v29.0.0 elastic/kibana#77802

Merged

chandlerprall mentioned this pull request Sep 29, 2020

Upgraded EUI to v29.3.0 elastic/kibana#78870

Merged

[EuiContextMenu] Add line separator as menu item #4018

[EuiContextMenu] Add line separator as menu item #4018

Uh oh!

Conversation

streamich commented Sep 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

kibanamachine commented Sep 8, 2020

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Sep 10, 2020

Uh oh!

cchaos commented Sep 10, 2020

Uh oh!

kibanamachine commented Sep 10, 2020

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

streamich commented Sep 14, 2020

Uh oh!

kibanamachine commented Sep 14, 2020

Uh oh!

chandlerprall commented Sep 14, 2020

Uh oh!

kibanamachine commented Sep 14, 2020

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chandlerprall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Sep 14, 2020

Uh oh!

kibanamachine commented Sep 15, 2020

Uh oh!

chandlerprall left a comment

Choose a reason for hiding this comment

Uh oh!

streamich commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

streamich commented Sep 17, 2020

Uh oh!

chandlerprall commented Sep 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

streamich commented Sep 8, 2020 •

edited

Loading

streamich commented Sep 17, 2020 •

edited

Loading