Refactor map component - Githubissues

laudickson commented 2 years ago

First few thoughts trying out a refactored more 'general' component for map

Changed from vanilla js mapbox library to react-map-gl pacakge. I've found it is WAY more react-typescript friendly and felt very little friction with the type-system

This is pretty rough and tbh the more I played with ReactMapGL I questioned if even it's necessary to have a generic in-app Map component and just use ReactMapGL component directly when you need to - their API and documentation is pretty comprehensive and they give lots of examples that offers a lot of great patterns

https://visgl.github.io/react-map-gl/examples/geojson https://visgl.github.io/react-map-gl/docs

Also used some things from Brad's PR https://github.com/codeforboston/mattapan-mapping/blob/b909bec69422fbd94183af1a95c88824305d8511/src/components/Map/Map.tsx v useful

This PR is in draft for now, I'll spend a couple more shower thoughts and see if I can polish this further. The 'Explore' page is playground for now to see how an instance of Map component could play out (still using not-curated-data)

netlify[bot] commented 2 years ago

✔️ Deploy Preview for mattapan-mapping ready!

🔨 Explore the source changes: a7f1356ebd295c3252dc74e86b8bc6d5c9773fda

🔍 Inspect the deploy log: https://app.netlify.com/sites/mattapan-mapping/deploys/618c1ec7c6e11a000944ea89

😎 Browse the preview: https://deploy-preview-69--mattapan-mapping.netlify.app/

laudickson commented 2 years ago

Also, I deleted package.json in this changeset incase things conflate with yarn packages (I think I had it from when originally I set up CRA)

thadk commented 2 years ago

Thanks for choosing our opinion(s) on the Mapbox library setup, @laudickson!

Not sure why the deploy preview situation is so overloaded but this is the one that built I think, though the mapbox token might not be right still https://deploy-preview-69--mattapan-mapping.netlify.app/#/ Do you happen to have 1-3 netlify's going in your account there?

Final build was most successful I guess: https://mattapan-mapping.netlify.app/explore

laudickson commented 2 years ago

I'll go over and share more in-depth with group tomorrow, but I'm somewhat settling on this pattern for now -

All Map-like components in MapAtoms; I don't think having Map in organism makes sense anymore
Whenever you want to make a map with a geojson data-type (or multiple geojsons), the geojson you want to import has to have a corresponding 'layer' with it; basically a json that describes how the geojson will be styled (follows LayerProp as described by react-map-gl`

Ex. pattern loosely looks like

import geoJsonData1 from './geoJsonData1';
import geoJsonData2 from './geoJsonData2';

const layer1 = { id: 'foo', type: 'line', paint: { 'line-color': 'orange' } };
const layer2 = { id: 'bar', type: 'fill', paint: {fill-color': 'purple' } };

      <Map mapStyle={ Theme.map.dark }>
        <>
          <MapSource data={ geoJsonData1 } layer={ layer1 } />
          <MapSource data={ geoJsonData2 } layer={ layer2} />
        </>
      </Map>

Concerns/issues:

Type casting geojson data before able to actually put into as prop value (omitted in example)
necessary fragment brackets because React children only expects 1 child (how to get around?)
questioning the whole pattern itself / appropriate in MapAtoms?
CRA doesn't allow imports outside of src directory; meaning assets will have to be moved into that (and potentially renamed and change to .ts extension?)
Need to create new issue that creates control panel component for toggling a map source
- https://visgl.github.io/react-map-gl/examples/custom-cursor
- https://visgl.github.io/react-map-gl/examples/geojson

laudickson commented 2 years ago

Not sure why the deploy preview situation is so overloaded but this is the one that built I think, though the mapbox token might not be right still https://deploy-preview-69--mattapan-mapping.netlify.app/#/ Do you happen to have 1-3 netlify's going in your account there?

Final build was most successful I guess: https://mattapan-mapping.netlify.app/explore

@thadk good look there, it has been working locally for me pretty much all this time but turns out it was related to this https://github.com/visgl/react-map-gl/issues/1266 and this version of mapbox isn't transpiring correctly. Lots of suggested solutions in that thread, but this one seemed to work as far as from looking at the deploy preview (craco ftw once again!)

laudickson commented 2 years ago

and move assets and boundaries into public folder and use from there

codeforboston / mattapan-mapping

Refactor map component #69