zoom to all fiscal polygons, not only current year, add helper methods to handle activity boundaries #584
Conversation
|
yzlucas
changed the title
| } | ||
| }; | ||
|
|
||
| const result = (component as any).mapFiscalActivities(response, fiscal); |
There was a problem hiding this comment.
Try to avoid using "any". You can always define a type and use that instead so it's explicit and we're not just letting anything through. This can also help with error checking
| (this.map.zoomControl as L.Control.Zoom).setPosition('topright'); | ||
| (this.map.zoomControl).setPosition('topright'); | ||
|
|
||
| L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', { |
There was a problem hiding this comment.
Would be good to parameterize this. It isn't likely to change much or at all, but just in case. Should we be sticking with openstreetmap or using DataBC BC basemaps?
| ); | ||
| } | ||
|
|
||
| private mapActivityBoundary(boundary: any, activity: any): any { |
There was a problem hiding this comment.
You have a could to types here defined. boundary object, whatever that is, or the manually defined type, or null. Rather than any define these types and set the return to be : type | type | null vs : any
There was a problem hiding this comment.
Sounds good, I will create some objects in model.ts to store them. Will go through the application to use defined type with updated method signature. I will create a separate to address them
| })); | ||
| } | ||
|
|
||
| private handleActivitiesResponse(allActivities: any[]): void { |
| ); | ||
| } | ||
|
|
||
| private mapFiscalActivities(response: any, fiscal: any): any[] { |
No description provided.