search

stamen / mapbox-gl-style-diff

Other
2 stars 1 forks source link

url-switching UI + styling updates #5

Closed peterqliu closed 2 years ago

peterqliu commented 2 years ago

Miscellaneous style updates, plus new text inputs to configure lstyle and rstyle url's.

Link. closes https://github.com/stamen/mapbox-gl-style-diff/issues/2

Screen Shot 2022-01-07 at 11 35 52 AM

cc/ @mizmay @kelsey-taylor

peterqliu commented 2 years ago

breaking more of this app out into separate files and testing individual diffing functions

can you say more about what you mean by individual diffing functions? Ideally this tool will persist for all cases we want to track stylesheet changes, but I don't see it getting significantly more complex than it is now.

Though diff.js is still a pretty coarse adaptation of what's from mapboxgl, and can use a light cleanup/refactor

aparlato commented 2 years ago

can you say more about what you mean by individual diffing functions?

Maybe "individual diffing functions" isn't necessarily the right term here, but I just mean to ask whether we should write a simple set of tests that confirm we get the diffs we expect to display. So like simple individual test cases for all the possible diffs we'd display, comparing styles where a layer is removed, added, etc and structuring the code to accommodate easy testing (which might be the case now). I think some level of testing is essential for this code since a breakage would likely not "break" the app, but instead give us bogus results.

aparlato commented 2 years ago

Also, @peterqliu I want to follow up and say, I definitely don't think we need to add tests/refactor in this PR specifically. That can be a followup issue, but I just want to get on the same page about the nature of this repo!

peterqliu commented 2 years ago

thanks for the feedback @aparlato -- implemented most of these, though let's follow up on the meatier ones (testing, refactoring the fetches, etc) in subsequent PR'S