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 geometry of selected BigCZ result to map #2213

Merged
merged 1 commit into from
Sep 5, 2017

Conversation

caseycesari
Copy link
Member

@caseycesari caseycesari commented Aug 30, 2017

Overview

When a user views the details of a BigCZ result, the geometry of the
selected result is highlighted on the map, and all of the other items
on the map are removed.

Connects #2152

Demo

bigcz4

Notes

I initially tried using the same layer group as the highlight layer, but because of the way the events are wired up for that layer, I wasn't able to do it. The mouseout event of the result item in the list would fire when the user moves the mouse over detail pane, and the highlight would be cleared.

Marker styling was based on the wireframes: https://marvelapp.com/5a66caa/screen/29866795

Testing Instructions

  • Start BiGCZ mode, draw an AoI, and make a search.
  • Select different results, and verify that when you do, the geometry for that result is the only marker on the map.

@rajadain
Copy link
Member

@azavea-bot rebuild

Copy link
Member

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

This works great for WDC. For CINERGI, if I select a result whose AoI is bigger than my viewport, I get the blue outline. But then, if I zoom out, I never get the actual shape, and only ever get the blue outline. Similarly, if I select a shape and then zoom in to it, instead of getting the blue outline when I'm too close, I get the blue overlay.

Can we add a zoom / pan handler to the map that updates this for the active shape when in the details view?

@@ -206,7 +216,8 @@ var MapView = Marionette.ItemView.extend({
'change:size': 'toggleMapSize',
'change:maskLayerApplied': 'toggleMask',
'change:dataCatalogResults': 'renderDataCatalogResults',
'change:dataCatalogActiveResult': 'renderDataCatalogActiveResult',
'change:dataCatalogActiveResult': 'renderDataCatalogResult',
'change:dataCatalogDetailResult': 'renderDataCatalogResult',
Copy link
Member

Choose a reason for hiding this comment

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

Since they both trigger the same function, consider putting them in one line:

'change:dataCatalogActiveResult change:dataCatalogDetailResult': 'renderDataCatalogResult',

otherwise it seems like there is a mistake in the first one, where the left and right side don't match.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, condensing them into one line modifies the shape of the event object. The changed attribute is no longer included in the event payload, which prevents the event handler from detecting which attribute is changing.

otherwise it seems like there is a mistake in the first one, where the left and right side don't match.

I'm not sure I understand. Can you elaborate?

} else {
this.detailsRegion.show(new ResultDetailsView({
model: detailResult,
activeCatalog: activeCatalog.id
}));
App.map.set('dataCatalogActiveResult', detailResult.get('geom'));
App.map.set('dataCatalogResults', null);
App.map.set('dataCatalogDetailResult', detailResult.get('geom'));
Copy link
Member

Choose a reason for hiding this comment

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

Consider combining these into one set:

App.map.set({
  dataCatalogResults: null,
  dataCatalogDetailResult: detailResult.get('geom'),
});

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Will do.

@caseycesari
Copy link
Member Author

Pushed a couple fixup commits to address the feedback.

Demo:

bigcz1

Copy link
Member

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 tested, works beautifully. Friendly reminder to squash before merging. CI errors are unrelated.

@rajadain rajadain assigned caseycesari and unassigned rajadain Aug 31, 2017
When a user views the details of a BigCZ result, the geometry of the
selected result is highlighted on the map, and all of the other items
on the map are removed.

Refs #2152
@caseycesari caseycesari merged commit 19a8342 into develop Sep 5, 2017
@caseycesari caseycesari deleted the cpc/add-selected-result-geom branch September 5, 2017 14:13
@rajadain rajadain mentioned this pull request Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants