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

[Maps] Add help menu #32561

Merged
merged 5 commits into from
Mar 7, 2019
Merged

Conversation

thomasneirynck
Copy link
Contributor

Adds help menu to the main help button

cf. #28412 (comment)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-docs

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link

@alexfrancoeur alexfrancoeur left a comment

Choose a reason for hiding this comment

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

Just tested out, LGTM. Do we want to point the @nreese getting started docs instead? #32431

@@ -0,0 +1,38 @@
/*
Copy link
Contributor

@nreese nreese Mar 6, 2019

Choose a reason for hiding this comment

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

Not sure if this file should go here. public/components is for redux connected components. Maybe this file should go in public/shared/components?

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 public/components should be renamed to public/connected_components to make that clear but that is for another issue

@@ -205,6 +206,13 @@ app.controller('GisMapController', ($scope, $route, config, kbnUrl, localStorage
{ text: $scope.map.title }
]);

chrome.helpExtension.set(domElement => {
Copy link
Contributor

@nreese nreese Mar 6, 2019

Choose a reason for hiding this comment

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

Maybe this extension point also needs to be set in https://github.com/elastic/kibana/blob/master/x-pack/plugins/maps/public/index.js so that the help context is also displayed on the listing page

screen shot 2019-03-06 at 10 50 20 am

@nreese
Copy link
Contributor

nreese commented Mar 6, 2019

Do we want to point the getting started docs instead?

I vote to point to the top of the maps documentation

@gchaps
Copy link
Contributor

gchaps commented Mar 6, 2019

I also vote for the top of the Maps documentation.

@alexfrancoeur
Copy link

Works for me!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck thomasneirynck requested a review from nreese March 7, 2019 16:18
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
code review, tested in chrome

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck thomasneirynck merged commit cbe1148 into elastic:master Mar 7, 2019
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Mar 7, 2019
@thomasneirynck
Copy link
Contributor Author

thomasneirynck commented Mar 7, 2019

needs backports to 6.7 and 7.0 branches [EDIT, remove 6.7 branch]

@nreese
Copy link
Contributor

nreese commented Mar 7, 2019

Can this be backported to 6.7? I don't think the link to docs exists in 6.7 since its using the old chrome

@alexfrancoeur
Copy link

alexfrancoeur commented Mar 7, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants