Slog through app errors to achieve parity with upstream OSM Website

[danrademacher]

Over in #198 we have gotten to the point of a sort-of working branch of the OHM website with the latest OSM website code, but we need to sort through application errors and figure out places where we need to reinstate changes we made to eventually have the site working based on the latest OSM website code.

@Rub21 at DevSeed has put together this documentation, which @batpad and I road tested and updated, to run the site locally: https://github.com/OpenHistoricalMap/ohm-deploy/tree/new_web_version/images#setting-up-ohm-website-for-development-mode

In wonderful news, Ruben has implemented a method here where changes in Rails code/assets are immediately recognized. Hallelujah! No more 10 minute local builds.

So my hope is that is some compensation for the slogging part of this of loading up the site, seeing bugs and tracking them down.

This is as far as we have gotten so far:

This is not a super urgent task per se, but it would be nice to move through next week or two so the upstream OSM doesn't get too far out of alignment.

[gregallensworth]

• [x] L.OSM.CyclOSM is not a constructor fatal error on startup

In the current osm-website, this is defined in vendor/assets/leaflet/leaflet.osm.js https://github.com/openstreetmap/openstreetmap-website/blob/70d7d8d850148f714b70a3297c02a8203214dec6/vendor/assets/leaflet/leaflet.osm.js#L22

The OHM version of vendor/assets/leaflet/leaflet.osm.js lacked this.

Pasting in a copy from the OSM Github repo was easy and effective.

• [x] Map container is already initialized.

app/assets/javascripts/index.js initializes the L.OSM.Map which itself is defined in app/assets/javascripts/leaflet.map.js

  var map = new L.OSM.Map("map", {
    zoomControl: false,
    layerControl: false,
    contextmenu: true,
    worldCopyJump: true
  });

It does this TWICE at line 29 the very top and again at 91 after OSM.mapParams and the OSM.loadSidebarContent definition. The second one is OHM's, while OSM's is the first one.

I replaced OSM's at the top with OHM's version, which includes maxZoom, maxBounds, etc.

• [x] layers is not defined

In leaflet.map.js the updateLayers() function is defined. OHM's version has slightly differences, since it loops over "layerParams" while OSM's newer code calls it "layers"

I replaced this function with one that's identical to OSM's except for the default: layers = layerParam || "O" instead of it being M

• [x] wrong default layer on startup

At this point the Leaflet map comes up, showing tiles from https://tileserver.memomaps.de/ which is L.OSM.OPNVKarte

This new OSM adds a new layer with a code of "O" same as I had picked for OHM Historical originally. So it's picking this one instead of our own.

Solution: leaflet.map.js I commented out L.OSM.OPNVKarte so it's no longer claiming code O.

• [x] missing translations for the OHM vector layer names

[missing "en.javascripts.map.base.woodblock" translation] and [missing "en.javascripts.map.base.historical" translation]

Added two lines to config/locales/en.yml in the map / base section, alongside "Cycle Map" et al. around line 2830

Also did this for en-GB.yml

        historical: Historical
        woodblock: Woodblock

Then ran bundle exec rake i18n:js:export to "compile" them into the en.js and en-GB.js counterparts.

[gregallensworth]

• [x] querystring is not defined even when using a tile basemap, so surely not vector / timeslider related

Old ohm-website code app/assets/javascripts/application.js includes this library then defines a global by importing it:

//= require querystring
var querystring = require('querystring-component');

So:

• copy in the library itself vendor/assets/javascripts/querystring.js
• add those 2 lines to app/assets/javascripts/application.js
• in the container, re-run the precompile

[gregallensworth]

• [x] vector maps: Cannot read properties of undefined (reading 'expression') happens when trying to use one of the vector maps, or when opening the Map Layers which shows thumbnails of those maps

See updated version below.

[gregallensworth]

• [x] vector maps: Cannot read properties of undefined (reading 'expression') happens when trying to use one of the vector maps, or when opening the Map Layers which shows thumbnails of those maps

I see that mapboxgl.version returns 2.7.0 instead of the expected, much older 0.53.1 which was the last open source version, and the one expected by the timeslider et al.

I see that the package.json and node_modules/mapbox-gl/package.json are 0.53.1

But I see ./Gemfile.lock: mapbox-gl-rails (2.7.0) and I see at https://github.com/nbulaj/mapbox-gl-rails that 3 months ago they updated everything to 2.7.0

So:

• edit Gemfile to copmment out `# gem "mapbox-gl-rails"

• in container, bundle update

• then re-precompile and restart

• [ ] Why is Ruby on Rails using mapbox-gl at all? It's purely a frontend phenomeon, so Ruby shouldn't need to know anything about it... unless there are some other panels that I don't know about? @batpad @Rub21

[gregallensworth]

Basically working, but some problems.

• [x] Timeslider's bar is offset downward, goes past the bottom of the white area instead of having padding

Figured it out: no explicit line-height is given for .mbgl-control-timeslider .mbgl-control-timeslider-dateinput in the MBGL control.

The screen-ltr CSS stylesheet seems to have changed, from providing an explicit 20px which this was inheriting, to simply "inherit" which is then inheriting a 1.5 from someplace else. Result: the current-year input is 27px tall instead of 18px tall, pushing the bar down.

So this is new work for the TimeSlider v1 code, and I'll write up an issue for explicit line-height:

.mbgl-control-timeslider .mbgl-control-timeslider-dateinput
line-height: 20px;

• [x] Arabic and Hebrew labels are not rendering

I see that app/aseets/javascripts/ohm.style.original.js is using Open Sans Regular as its font. A few weeks back, we changed this out to "OpenHistorical" and there were other style updates as well. Solution: replaced file content with the updated one.

Also, ohm.style.js is lacking the RTL support that was patched in 16 days ago. Solution: added this paragraph.

[gregallensworth]

All righty! That was all of the obvious errors resulting from the OSM upgrade which prevented the vector maps from basically working. Other pages and functions could use some testing, and it's possible that there's more to test that doesn't occur to me offhand.

• [ ] click to query, then bringing up a specific relation from the click results

Click query hits Overpass over at https://overpass-api.de/api/interpreter and gets me a list of results.

Bringing up a specific result, though, hits localhost e.g. http://localhost/api/0.6/way/152363088/full and this fails.

ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR:  relation "current_ways" does not exist
LINE 9:  WHERE a.attrelid = '"current_ways"'::regclass

Not surprising, since I have just the starting DB with nothing in it. But worth testing, to see what else does / doesn't work here. Though, none of this part of OSM is about vector tiles and time slider so perhaps is beyond the scope of this issue?

• [ ] viewing with a specific relation ID in the URL

Example URL = http://localhost/way/152363088

Same error as above that current_ways does not exist.

[danrademacher]

We now have the upstream fork on Staging and can test everything there. I already see some issues and they are going to be a mix of things for GreenInfo and DevSeed (or maybe unclear at first), so I'm going to close this umbrella issue out and file specific items