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

KuiContextMenu => EuiContextMenu #17621

Merged

Conversation

stacey-gammon
Copy link
Contributor

@stacey-gammon stacey-gammon commented Apr 9, 2018

Before:
screen shot 2018-04-09 at 4 15 55 pm

After:
screen shot 2018-04-09 at 4 12 29 pm

I had to add a specific display:block on the euiContextMenuItem to get them to show up stacked. No idea why... design team might be able to help. :)

Oh I also need a collapse icon, right now I'm just using expand for both:
screen shot 2018-04-09 at 4 17 49 pm

@stacey-gammon stacey-gammon added Feature:Dashboard Dashboard related features :Sharing labels Apr 9, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@stacey-gammon stacey-gammon force-pushed the switch-to-eui-context-menu branch 2 times, most recently from 084ba77 to e8f2fee Compare April 9, 2018 20:50
@stacey-gammon stacey-gammon requested review from snide and nreese April 9, 2018 21:29
@@ -23,6 +23,10 @@
}
}

.euiContextMenuItem {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only thing I'm not sure of. Will see if @snide has any ideas.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nreese
Copy link
Contributor

nreese commented Apr 9, 2018

I could not rename a panel under "Customize Panel". The "Panel Title" input did not update when typed in.

Everything else worked great.

@stacey-gammon
Copy link
Contributor Author

Rebased with master, now everything should be working, the fix was in EUI.

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm

@elasticmachine
Copy link
Contributor

💔 Build Failed

@stacey-gammon
Copy link
Contributor Author

hm, failed on:

37:11     ✓ throws an error if currency is not three letter code
15:37:11 (node:667) UnhandledPromiseRejectionWarning: TypeError: type.toLowerCase is not a function
15:37:11     at Assertion.an (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-intake/kibana/node_modules/chai/lib/chai/core/assertions.js:165:17)
15:37:11     at Assertion.assert (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-intake/kibana/node_modules/chai/lib/chai/utils/addChainableMethod.js:84:49)
15:37:11     at catch.e (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-intake/kibana/src/core_plugins/timelion/server/series_functions/__tests__/yaxis.js:76:23)
15:37:11     at <anonymous>:null:null
15:37:11 
15:37:11 (node:667) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 4)
15:37:11 (node:667) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
15:37:11 (node:667) UnhandledPromiseRejectionWarning: TypeError: type.toLowerCase is not a function
15:37:11     at Assertion.an (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-intake/kibana/node_modules/chai/lib/chai/core/assertions.js:165:17)
15:37:11     at Assertion.assert (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-intake/kibana/node_modules/chai/lib/chai/utils/addChainableMethod.js:84:49)
15:37:11     at catch.e (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-intake/kibana/src/core_plugins/timelion/server/series_functions/__tests__/yaxis.js:79:23)
15:37:11     at <anonymous>:null:null

@stacey-gammon stacey-gammon force-pushed the switch-to-eui-context-menu branch from a209213 to e6a2f32 Compare May 10, 2018 17:07
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@stacey-gammon stacey-gammon merged commit 0679ab2 into elastic:master May 10, 2018
sorenlouv pushed a commit to sorenlouv/kibana that referenced this pull request May 10, 2018
* Switch to euicontextmenu

* update jest panel snapshots
@nreese
Copy link
Contributor

nreese commented May 16, 2018

@stacey-gammon I don't think this got backported to 6.x. It looks like the backport PR was closed and not merged.

stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request May 16, 2018
* Switch to euicontextmenu

* update jest panel snapshots
@stacey-gammon
Copy link
Contributor Author

oh no! good catch. Not sure how that happened. Will throw up a PR now.

@nreese
Copy link
Contributor

nreese commented May 16, 2018

I found it because I had a merge conflict with #19130

stacey-gammon added a commit that referenced this pull request May 16, 2018
* Switch to euicontextmenu

* update jest panel snapshots
@stacey-gammon stacey-gammon deleted the switch-to-eui-context-menu branch May 31, 2018 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features v6.4.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants