79 center zoom hash #187

chriswhong · 2017-09-27T20:52:47Z

This PR adds the mapboxGL map hash, allowing us to persist map views on page reload.

It also changes the fitbounds behavior

Transitioning from sub-routes to index no longer zooms out
Highlighted features are cleared when transitioning back to index
Fitbounds adds an offset option so the fit is correct even though the map is obscured behind the details pane.

TODO: Add logic to fitBoundsOptions to make sure it works on mobile

andycochran · 2017-09-27T21:31:05Z

The © info and map scale are hidden behind the content on ≥ large screens

andycochran · 2017-09-27T21:31:54Z

Seems like the map pan/zooms on route change on very large screens but not on smaller screens.

andycochran · 2017-09-27T21:33:45Z

This needs to account for the large and xxlarge CSS media queries:

$breakpoints: (
  small: 0,
  medium: 640px,
  large: 1024px,
  xlarge: 1200px,
  xxlarge: 1440px,
);

andycochran · 2017-09-27T21:35:01Z

To keep the media queries in sync with the JS consider using this method to have the CSS tell the JavaScript which media query is being used: http://zerosixthree.se/detecting-media-queries-with-javascript/

chriswhong · 2017-09-27T21:40:14Z

Will tackle tomorrow, not ready to merge yet

# Conflicts: # app/styles/layouts/_l-default.scss

allthesignals · 2017-09-27T21:34:54Z

app/components/main-map.js

@@ -43,8 +43,16 @@ export default Ember.Component.extend({
      const height = el.height();
      const width = el.width();

+      const fullWidth = $('.navigation-area').width();


@chriswhong is this the only div where we can get correct information about the width of the screen?

No, it's just the closest one. Just need something full width so we can calculate the percentage that the details pane will take up.

Got it - so I think window width will do.

allthesignals · 2017-09-27T21:45:50Z

app/components/main-map.js

+      return {
+        padding: selected && (type !== 'zoning-district') ? padding : 0,
+        offset: [offset, 0],
+      };


@chriswhong what should this computed be doing in terms of the view? Having trouble figuring out how to test / verify. Thanks!

^^ I have some ideas on how to approach this differently

This offsets the fitbounds, I am not sure how you would test the output, you would need to be able to read the centerpoint of the map vs the center of the highlighted polygon somehow.

Ah, I just mean manual testing. Why does zoning-district get padding but not others? Thanks!

allthesignals · 2017-09-27T21:47:22Z

app/components/main-map.js

+  @computed('mainMap.shouldFitBounds')
+  shouldFitBounds(shouldFitBounds) {
+    return shouldFitBounds;
+  },


I'll refactor this to a computed alias (shrink 4 lines of code into 1)

Basically, shouldFitBounds: alias('mainMap.shouldFitBounds')

allthesignals · 2017-09-27T21:53:54Z

app/routes/index.js

+    didTransition() {
+      this.get('mainMap').set('selected', null);
+      this.get('mainMap').set('shouldFitBounds', false);
+    },


I'll take care of this but if you're setting on a property you don't need to get it, you can just write:
this.set('mainMap.selected',null);

Alternatively, if there are multiple properties, you can just say:

this.get('mainMap') .setProperties({ selected: null, shouldFitBounds: false, });

good to know!

allthesignals · 2017-09-27T21:54:22Z

app/routes/lot.js

@@ -14,5 +14,6 @@ export default Ember.Route.extend({
  afterModel(model) {
    const mainMap = this.get('mainMap');
    mainMap.set('selected', model);
+    mainMap.set('shouldFitBounds', true);


Same as above, I'll take care of it tonight:

this.get('mainMap') .setProperties({ selected: null, shouldFitBounds: false, });

allthesignals · 2017-09-27T21:54:46Z

app/routes/zma.js

-      const application = this.controllerFor('application');
-      application.resetQueryParams();
+    didTransition() {
+      this.get('mainMap').set('shouldFitBounds', true);


this.set('mainMap.shouldFitBounds', true);

allthesignals · 2017-09-27T21:54:59Z

app/routes/zoning-district.js

+
+  actions: {
+    didTransition() {
+      this.get('mainMap').set('shouldFitBounds', true);


andycochran · 2017-09-28T14:12:46Z

I pulled down the latest on this and got a weird merge conflict. I fixed the _l-default.scss file, but GitHub says my commit touched other files. Please confirm I didn't break anything. 😬

…a into 79-center-zoom-hash

Resize was being called before a route transition finished (and actually affected the DOM) and after a fitBounds animation finished, meaning it never had the correct DOM context to resize properly. Two things had to happen: - ember-mapbox-gl’s call needed to be wrapped in an afterRender callback in the Run cycle so that things were synchronous - resize() must be called before fitBounds, but after a render has completed Fix is now in place globally, but resize is commented out for now pending some other decision.

Also don’t use private variables

In favor of 2 commits ago

…a into 79-center-zoom-hash

allthesignals · 2017-09-28T18:22:40Z

@chriswhong @andycochran
I finally successfully solved the resize issue which is described in the commit
(4284b18). However, it seems like the strategy of this PR was to solve it by working around it, and making the map absolutely positioned so that .content-area would float over it. That means that even though the original behavior works (resize map when a new div is inserted), the map center still jumps to adjust for the container's new width, moving layers and content along with it.

There is a long discussion thread on this related issue here: mapbox/mapbox-gl-js#1740. Basically, MapboxGL does not yet provide a way to globally offset the map's internal concept of a "center", although it does provide an "around" option (https://www.mapbox.com/mapbox-gl-js/api/#cameraoptions) for a select few panning/zoom methods.

I think this will have to be the working solution for now but we'll need to find a more organized way to deal with the layout widths to calculate offsets since that will cause issues for mobile and other views.

Chris Whong added 2 commits September 27, 2017 15:55

WIP calculated fitbounds offset

a6ea3e1

add url hash, fitbounds only on non-route transitions

9f781fd

ghost assigned chriswhong Sep 27, 2017

ghost added the in progress label Sep 27, 2017

Fix tests

3771556

ghost assigned allthesignals Sep 27, 2017

move absolute position of content area into large media query

cfae714

Merge branch 'develop' into 79-center-zoom-hash

df33061

# Conflicts: # app/styles/layouts/_l-default.scss

chriswhong mentioned this pull request Sep 27, 2017

Persist pitch/easing/lat/lng/zoom into URL #79

Closed

update package-lock.json

afc0c01

allthesignals reviewed Sep 27, 2017

View reviewed changes

fix merge conflict

0ec57a4

ghost assigned andycochran Sep 28, 2017

allthesignals added 10 commits September 28, 2017 11:12

Clean up

69f4911

Merge branch '79-center-zoom-hash' of github.com:NYCPlanning/labs-zol…

56e6d5d

…a into 79-center-zoom-hash

Remove ember-resize

5617359

Remove resize-aware as it was unneeded

8cf69da

Also don’t use private variables

Remove old resize logic

3653adf

In favor of 2 commits ago

Refactor some code

f2db773

Merge branch 'develop' into 79-center-zoom-hash

465a942

Fix test

98656f5

Merge branch '79-center-zoom-hash' of github.com:NYCPlanning/labs-zol…

5690798

…a into 79-center-zoom-hash

update padding calculation

ec2d256

allthesignals mentioned this pull request Sep 28, 2017

Revisit isSelectedBoundsOptions calcs #195

Closed

allthesignals merged commit 22222f6 into develop Sep 28, 2017

ghost removed the in progress label Sep 28, 2017

chriswhong mentioned this pull request Sep 28, 2017

Fitbounds Changes #154

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

79 center zoom hash #187

79 center zoom hash #187

chriswhong commented Sep 27, 2017

andycochran commented Sep 27, 2017 •

edited

Loading

andycochran commented Sep 27, 2017

andycochran commented Sep 27, 2017

andycochran commented Sep 27, 2017

chriswhong commented Sep 27, 2017

allthesignals Sep 27, 2017

chriswhong Sep 28, 2017

allthesignals Sep 28, 2017

allthesignals Sep 27, 2017

allthesignals Sep 27, 2017

chriswhong Sep 28, 2017

allthesignals Sep 28, 2017

allthesignals Sep 27, 2017

allthesignals Sep 27, 2017

allthesignals Sep 27, 2017 •

edited

Loading

chriswhong Sep 28, 2017

allthesignals Sep 27, 2017 •

edited

Loading

allthesignals Sep 27, 2017

allthesignals Sep 27, 2017

andycochran commented Sep 28, 2017

allthesignals commented Sep 28, 2017 •

edited by andycochran

Loading

79 center zoom hash #187

79 center zoom hash #187

Conversation

chriswhong commented Sep 27, 2017

andycochran commented Sep 27, 2017 • edited Loading

andycochran commented Sep 27, 2017

andycochran commented Sep 27, 2017

andycochran commented Sep 27, 2017

chriswhong commented Sep 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allthesignals Sep 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allthesignals Sep 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andycochran commented Sep 28, 2017

allthesignals commented Sep 28, 2017 • edited by andycochran Loading

andycochran commented Sep 27, 2017 •

edited

Loading

allthesignals Sep 27, 2017 •

edited

Loading

allthesignals Sep 27, 2017 •

edited

Loading

allthesignals commented Sep 28, 2017 •

edited by andycochran

Loading