dcfemtech / hackforgood-waba-map

DCFemTech Hack for Good 2016 - WABA Bike Map Project
MIT License
10 stars 9 forks source link

Mapbox GL JS migration #97

Closed hrecht closed 7 years ago

hrecht commented 7 years ago

New gl-js branch based on the latest master - this addresses https://github.com/dcfemtech/hackforgood-waba-map/issues/58, https://github.com/dcfemtech/hackforgood-waba-map/issues/89, and https://github.com/dcfemtech/hackforgood-waba-map/issues/88

I wasn't sure what the goal of the start/end section is - is the ultimate goal to draw a route between the points? Or something else? I included the directions plugin assuming the goal is cycling directions. Clicking on the map will drop a pin there, geocode, and add it to the directions box. screen shot 2016-08-20 at 5 33 32 pm Users can also type addresses into the directions box.

I removed the start/end divs assuming the goal was those directions. I think the location box can now be removed as well, but wasn't sure if that box served an additional purpose - I moved it to the left to accomodate the directions. screen shot 2016-08-20 at 5 34 44 pm screen shot 2016-08-20 at 5 35 23 pm

The UI/layout will now need some updating for the visual hierarchy to make sense, but the functionality is in place. That would be a non-minor DOM/CSS update, so I'm not including in this PR. Relatedly:

Features from master that are not implemented yet:

One noticeable change: all bike lanes are visible on load. I found it confusing as a user to see a blank map on load. This can be easily changed in https://github.com/dcfemtech/hackforgood-waba-map/blob/gl-js-migration/site.js#L138 if needed.

And one question: what's the use case for displaying each county individually? Particularly for the buffers? There either is or isn't a bike lane within 500 meters (or 1 mile, etc), so seeing overlapping buffers at county edges is something that I'm not sure how to process as a user.

Note: there's a compatibility issue between mapbox-gl.js v0.22 and the directions/geocoding plugins. They've fixed in https://github.com/mapbox/mapbox-gl-directions/issues/82 but haven't put the dist on the mapbox website yet, so I used v0.21 for now.

alulsh commented 7 years ago

@hrecht - I'm planning on reviewing your code tonight at Code for DC but wanted to add some comments on your issue for now!

I wasn't sure what the goal of the start/end section is - is the ultimate goal to draw a route between the points? Or something else?

Hmmm @tomBeach added this as part of his refactor.

From what I understand WABA just needs a map visualization of the buffers for all the bike routes in DC. They may or may not want directions, but this is not the end goal. Can you chime in @nellepierson?

I think the location box can now be removed as well

I agree we can remove this - not useful for the audience of this visualization.

One noticeable change: all bike lanes are visible on load. I found it confusing as a user to see a blank map on load.

Yes! Great change šŸŽ‰ I agree all bike lanes should be visible on load.

And one question: what's the use case for displaying each county individually? Particularly for the buffers? There either is or isn't a bike lane within 500 meters (or 1 mile, etc), so seeing overlapping buffers at county edges is something that I'm not sure how to process as a user.

Agree, we should fix this - there's no use case. I'm thinking we might be able to use turf-js in the browser to do this and combine the buffers via a manual union. Turf-js crashed when we tried to do a manual union for 1500+ features, but shouldn't crash for doing a union of 5 buffers.

hrecht commented 7 years ago

Awesome, thanks! I think then the most important functions would be: 1. turn on/off buffers at the various distances (for all counties) and potentially 2. show cycling directions. I haven't used turf-js, but if you combined all the county bike lanes files and created the buffers based on a unified file that should theoretically work. That would also signficantly slow down load time, if there's one single bike lanes json and one buffer json for each distance vs 5 files per county. The bike lanes json could have a county fips property if there's a need to filter the map view by county (but it sounds like there isn't).

alulsh commented 7 years ago

Nice work @hrecht!!! Just merged in @tomBeach's branch. This looks great. I'm merging šŸŽ‰