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 View Control displaying coordinates at mouse position #28023

Merged
merged 8 commits into from
Jan 4, 2019

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Jan 3, 2019

Summary

This PR creates a new ViewControl that displays the coordinates of the mouse. The PR also moves the Set View button from the LayerControl into the ViewControl.

The PR cleans up the director structor of public/components by moving layer_control, layer_toc, and set_view under widget_overlay folder since that is where they are used and public/components is getting rather messy.

Setting state so often via SET_MOUSE_COORDINATES highlighted an existing timing issue that results in map panning and zooming unexpectedly snapping back to its previous location. The bug was that the store extent state was only updated when moveend was triggered. The problem was caused if state was changed before moveend could fire. The original implementation of _syncMbMapWithMapState would update map center and zoom with the stale values from the store instead of current values from mapbox. This PR fixes that bug as well since it made it happen all the time.

screen shot 2019-01-03 at 4 30 46 pm

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@nreese nreese requested a review from a team as a code owner January 3, 2019 20:06
@nreese nreese changed the title Mouse position [Maps] add View Control displaying coordinates at mouse position Jan 3, 2019
@nreese
Copy link
Contributor Author

nreese commented Jan 3, 2019

@cchaos Could I please get some design help. I need help moving the ViewControl to the bottom of the page and am not sure if my CSS classes are named according to guidelines.

screen shot 2019-01-03 at 1 06 25 pm

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Some random small stuff i saw on this one.

@@ -34,21 +34,24 @@ map-listing, .gisListingPage {
flex-grow: 1;
}

.LayerControl {
.widgetOverlay {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you can, prefix your css with your app.... I'd recommend .mapWidgetOverlay or .gisWidgetOverlay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will you gis even though the app is called map because gis is more unique and this is not user facing

Copy link
Contributor

Choose a reason for hiding this comment

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

@nreese Are you going to change these soon? I'm almost ready to submit my pr for you

padding-bottom: 8px;
border-color: transparent;

&.euiPanel--shadow {
@include euiBottomShadowLarge;
}

.LayerControl--header {
.WidgetControl--header {
padding: 16px 16px 8px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use eui sizes

@@ -64,7 +64,6 @@ export class MBMapContainer extends React.Component {
const initialZoom = this.props.mapState.zoom;
const initialCenter = this.props.mapState.center;
this._mbMap = await createMbMapInstance(this.refs.mapContainer, initialZoom, initialCenter);
window._mbMap = this._mbMap;
Copy link
Contributor Author

@nreese nreese Jan 3, 2019

Choose a reason for hiding this comment

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

Not sure why _mbMap was ever exposed on window but this should not be so I removed it

@nreese nreese added the WIP Work in progress label Jan 3, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@cchaos
Copy link
Contributor

cchaos commented Jan 3, 2019

PR4U nreese#28

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nreese nreese requested a review from thomasneirynck January 4, 2019 17:19
@nreese nreese added [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation and removed WIP Work in progress labels Jan 4, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis

@elasticmachine
Copy link
Contributor

💔 Build Failed

@nreese
Copy link
Contributor Author

nreese commented Jan 4, 2019

flaky x-pack tests X-Pack SAML API Integration Tests.x-pack/test/saml_api_integration/apis/security/saml_login·js

jenkins, test this

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

tested. nice eye for detail, e.g. readout goes blank when mouse leaves the map.

@nreese nreese merged commit 858b61b into elastic:master Jan 4, 2019
nreese added a commit to nreese/kibana that referenced this pull request Jan 4, 2019
…stic#28023)

* put mouse position in store

* widget overview component

* Fixing layout of overlay (#28)

* move layer_control and layer_toc under widget_overview folder

* clear mouse coordinates when mouse leaves map

* change how settig map view works to avoid state timing bug

* debounce moveend event
nreese added a commit that referenced this pull request Jan 4, 2019
) (#28104)

* put mouse position in store

* widget overview component

* Fixing layout of overlay (#28)

* move layer_control and layer_toc under widget_overview folder

* clear mouse coordinates when mouse leaves map

* change how settig map view works to avoid state timing bug

* debounce moveend event
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 v6.7.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants