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 geography and polygon choosers #67

Merged
merged 1 commit into from
Aug 11, 2015
Merged

Conversation

kshepard
Copy link
Member

State is currently propagated via stateParams in the URL. Once we have user account modules in place, it may make more sense to store these parameters there.

State is currently propagated via stateParams in the URL.
Once we have user account modules in place, it may make
more sense to store these parameters there.
@@ -2,45 +2,24 @@
'use strict';

/* ngInject */
function NavbarController($log, $rootScope, $state, $stateParams, RecordTypes) {
function NavbarController($log, $rootScope, $state, $stateParams,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there might be benefit in making a service to handle the responsibility for tracking the selected and available Geographies, Polygons, and RecordTypes. That would potentially simplify setResourceObjects because you would be able to remove the logic that sets the controller values via the $stateParams since a service would be less transient. It would also make things easier for other parts of the app that might need to respond to changes in the selected types, which several of them might.

Copy link
Member Author

Choose a reason for hiding this comment

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

I definitely agree that some abstraction here would be beneficial. I had a couple different ideas in mind that would depend on how we'd like to interface with these values in other parts of the app. Since it's not trivial, I'm going to make an issue for it, so we can discuss the best approach and then implement it in the next Sprint.

@ddohler
Copy link
Contributor

ddohler commented Aug 11, 2015

Consider making a service to track Geographies / Polygons / RecordTypes. Looks good otherwise. +1

kshepard added a commit that referenced this pull request Aug 11, 2015
@kshepard kshepard merged commit 40796d4 into develop Aug 11, 2015
@kshepard kshepard deleted the feature/boundary-chooser branch August 11, 2015 14:15
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.

2 participants