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

Add EMS hot link #21154

Merged
merged 3 commits into from
Jul 26, 2018
Merged

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Jul 24, 2018

This adds a dynamic hotlink for the EMS documentation to the EMS landing page for EMS vector layers selected in the vector layer drop down of region maps.


Example (NOTE: these images are

For EMS data:

image

For non-EMS data (no change):

image

@thomasneirynck thomasneirynck added Team:Docs Feature:Visualizations Generic visualization features (in case no more specific feature label is available) [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v6.4.0 labels Jul 24, 2018
@@ -21,6 +21,12 @@
</div>
</div>

<div class="kuiSideBarFormRow" ng-hide="!editorState.params.emsHotLink">
<div class="kuiSideBarFormRow__control">
Preview <a ng-href="{{editorState.params.emsHotLink}}" target="_">{{editorState.params.selectedLayer.name}} on the Elastic Maps Service</a>
Copy link
Member

Choose a reason for hiding this comment

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

As we flesh out the UI are we expecting this to be a similar UX to the icon-tooltip as seen in some of the other fields on this page? Or does including a link tag make it too complicated for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed, this needs to be improved. Icon-tooltip might help (e.g. similar to hot-linking the ES-doc for each bucket-aggregation

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

jenkins, test this

emsLandingPageUrl: Joi.when('$dev', {
is: true,
then: Joi.string().default('https://maps-staging.elastic.co/v2'),
otherwise: Joi.string().default('https://maps.elastic.co/v2')
Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed with @thomasneirynck previously, the current manifest used to populate the dropdown reports slightly different layers than what's available at the current EMS endpoints, for example 'USA Counties' is listed as available via the manifest, but the dynamically generated link with that layer name argument provided doesn't actually go to the resource. This is a known mismatch originally identified by @nickpeihl, and is something we can tackle either as a part of this issue or it could be a separate issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confident this will be a separate issue and doesn't necessarily have to hold up merging for now so long as the external services are updated and we update the manifest used for the dropdown and the generated link. I ran the code locally with no apparent issues. LGTM!

Copy link
Member

Choose a reason for hiding this comment

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

This is a separate issue, mainly because I misunderstood how the EMS landing page deployment works. maps-staging.elastic.co/v2 is showing the production manifest rather than the staging manifest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we decided to just have Kibana use production-EMS all the time, even in dev/master. #21237 addresses that (and should be merged before this one).

Copy link
Member

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

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

I'd still like to see input from the design team on how the link appears, but functionally, this is perfect.

@thomasneirynck
Copy link
Contributor Author

@nickpeihl agreed, will run this by design team

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cchaos
Copy link
Contributor

cchaos commented Jul 25, 2018

The link is a little long-winded. I don't think it's necessary to duplicate the name of the layer in the link, it can just always read "Preview on the EMS" and then add either a tooltip or a title tag with a little bit more explanation (including the full EMS acronym definition.

I'd then reduce the font-size, which isn't really easily done with generic tags, so I'd just add:

.vis-editor-config .ems-hotlink {
  font-size: 12px;
}

The extra selector is just to ensure scoping

But the functionality works and the zooming when you land on the EMS page is a really nice touch!

@thomasneirynck
Copy link
Contributor Author

thanks for feedback @cchaos, will make the changes..

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck
Copy link
Contributor Author

I rebased this with master, since this was blocked by #21237. I also made the corresponding change to always point at the production URL of the landing page.

@thomasneirynck thomasneirynck merged commit 99d1729 into elastic:master Jul 26, 2018
@thomasneirynck
Copy link
Contributor Author

needs to wait for backports #21291, #21292 to clear before backporting this.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jul 26, 2018
Adds a dynamic hyperlink to the EMS landing page for EMS vector layers selected in the vector layer drop down of region maps.
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jul 27, 2018
Adds a dynamic hyperlink to the EMS landing page for EMS vector layers selected in the vector layer drop down of region maps.
thomasneirynck added a commit that referenced this pull request Jul 27, 2018
Adds a dynamic hyperlink to the EMS landing page for EMS vector layers selected in the vector layer drop down of region maps.
thomasneirynck added a commit that referenced this pull request Jul 27, 2018
Adds a dynamic hyperlink to the EMS landing page for EMS vector layers selected in the vector layer drop down of region maps.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Feature:Visualizations Generic visualization features (in case no more specific feature label is available) Team:Docs v6.4.0 v6.5.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants