search

colouring-cities / colouring-britain

Developed out of the Colouring London prototype. Collecting data on Britain's buildings and testing new core features
https://colouringbritain.org/
GNU General Public License v3.0
10 stars 2 forks source link

Merge changes from Colouring Core #322

Closed mdsimpson42 closed 6 months ago

mdsimpson42 commented 6 months ago

The latest changes from Colouring Core, to be merged into Colouring Britain.

mdsimpson42 commented 6 months ago

@matkoniecz Can I have a sanity check on this? I'm trying to merge the latest changes from Colouring Core (master) to Colouring Britain (master), but there were some conflicts.

In particular, can you double-check "map.tsx" and "map-button.css", as those were the conflicting files?

I think I've set it all up correctly, but I just wanted to make sure (especially after I messed it up last time!)

matkoniecz commented 6 months ago

@mdsimpson42 Conflict itself sadly was to be expected (I had another idea how to apply it, but it would cause even worse conflict).

From what I see in map.tsx in switcher list

                            <AerialPhotosMapSwitcher/>
                            <HistoricalFootprintsSwitcher/>
                            <OpenStreetMapSwitcher/>

have become lost after this merge is done (between HistoricDataSwitcher and VistaSwitcher). This does not seem intentional.

matkoniecz commented 6 months ago

map-button.css looks like I would expected.

In https://github.com/colouring-cities/colouring-britain/blob/bcfdc473d89d7e29ab44d6568565de27dc77aa84/app/src/frontend/map/openstreetmap-switcher.tsx openstreetmap-switcher manually applied style has become not necessary but it can be followup PR.

I expect the same for AerialPhotosMapSwitcher and HistoricalFootprintsSwitcher.

matkoniecz commented 6 months ago

Note: I have only looked at code, not run it.

mdsimpson42 commented 6 months ago

Okay, I've made some updates that seem to have fixed the behaviour of the Switchers. I've tested the code and it all seems to be working. Will merge and deploy to Staging.

matkoniecz commented 6 months ago

oh yes, removal of https://github.com/colouring-cities/colouring-britain/pull/322/commits/4cd796216ac333d7cddade310aba0d2c9bba92ea was also needed as manually defining offset is not needed anymore