dcfemtech / hackforgood-waba-map

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

User filters #66

Closed tomBeach closed 8 years ago

tomBeach commented 8 years ago

Crude filter menu for selecting region (i.e. DC, MontCty...) and buffer size displays/hides selections on map on new branch "userFilters"

alulsh commented 8 years ago

Your UI of this at https://tombeach.github.io/hackforgood-waba-map/ is beautiful!

This PR however changes 181 files and 38,454 lines of code. Looking at your fork at https://github.com/tomBeach/hackforgood-waba-map, I can see you added bower (with chosen, font-awesome, jquery, and normalize-css libraries) along with lodash to this project. I can see in site_tb.js you are also using ES6.

The above changes add considerable dependency bloat and complexity to this project which I'm actively trying to avoid. This project is supposed to be lightweight and either embedded on the WABA website or hosted directly on their WordPress site.

Can you refactor this PR to instead use a single jQuery file such as the production, ready 2.2.4 minified version at https://code.jquery.com/jquery-2.2.4.min.js instead of a directory with several sub-directories? Same thing goes for lodash - can you refactor to use the production-ready, minified version at https://cdn.jsdelivr.net/lodash/4.14.2/lodash.min.js instead of an entire directory? Same thing for goes normalize-css, font-awesome, and any other libraries used in this project.

Re: ES6 - I will need to double check with WABA, but I would prefer to use traditional JavaScript instead of ES6 due to legacy browser support. I have a feeling we will need to support IE11, which does not support ES6. Can you rewrite this PR to use traditional JavaScript?

tomBeach commented 8 years ago

Definitely using an "include-by-habit" approach here! I'll work on eliminating lodash and the other bower file dependencies, with just jquery min version locally. Meanwhile, there's an updated version with buffer fixes and improved functionality at (https://tombeach.github.io/hackforgood-waba-map/).

alulsh commented 8 years ago

Closing in favor of https://github.com/dcfemtech/hackforgood-waba-map/pull/78, which will be much easier to merge 🙇 .