daat-hamakom / maps

Da'at Hamakom Frontend Map Interface
Other
0 stars 0 forks source link

labels of maps do not appear or not according to zoom level #146

Open josefspr opened 8 years ago

josefspr commented 8 years ago

Modernism: Jerusalem, Tel Aviv appear only in high zoom New York appears only in extremely low zoom Odessa does not appear at all Pre-Industrial: London does not appear (if missing can you add it ?)

yuvadm commented 8 years ago

@mushon might need to set icon-allow-overlapping: true (or whatever it's called)

mushon commented 8 years ago

I think it's actually the event icons that are causing these problems, can you @yuvadm try toggling that icon-allow-overlapping: true from your end.

yuvadm commented 8 years ago

@mushon this layer isn't currently managed by me so i prefer not to set any properties on it. can you just update the style with this property?

mushon commented 8 years ago

The layer that needs to get that change is actually the markers layer which is managed by you

yuvadm commented 8 years ago

@mushon I only change the source on it, otherwise I'm not adding it to the map or doing anything else with it. I have no problem changing the prop, but it's a bit out of place for me to do it instead of it just being on the style in the first place.

mushon commented 8 years ago

What needs to change is not the text label layer but the event markers layer, it is what appears above the labels and overlaps with them. Call me if this is still not making sense :)

mushon commented 8 years ago

@yuvadm As a proof that the markers are to blame, here's modernism the map w/o the markers: image

And with the markers: image

yuvadm commented 8 years ago

@mushon not sure what these screenshots show, but on a loaded map, when running

map.getLayer('markers').setLayoutProperty('visibility', 'none')

the markers went away but label rendering didn't change. (Note that you can play with the map via API, map is exposed globally on the window for debugging purposes)

mushon commented 7 years ago

@yuvadm please look into this issue again. I think we are not having as much rendering problems, but I am not sure all the location names are appearing as they should.

mushon commented 7 years ago

I looked into this again from my end and I think the reason for these glitches is the way we use the data. Rather than using all of the events to draw the labels we should be using another API call with one entry per city. For example, we have 144 entries for Tel Aviv, when we could go with a maximum of 5 - one for each map period. @yuvadm please test this, in either way it would better allow us to solve this issue. Thanks!

josefspr commented 7 years ago

@yuvadm would you please paste the link to the labels API calls so we can make sure that they are indeed being called? Please include the general call (all events) and broken by period (preindustrial, nineteenth, modern…) Thanks!

yuvadm commented 7 years ago

@josefspr there isn't really an API call for the labels, they are generated by processing the events and extracting all places from them, the relevant code is at https://github.com/daat-hamakom/maps/blob/master/src/components/map.js#L172-L202

mushon commented 7 years ago

So how can we make sure the data is generated? For example in the case of showing the Berlin label in the Modernist maps.

yuvadm commented 7 years ago

@mushon you can inspect the JS console and look at the generated places GeoJSON. Why do you suspect Berlin/Modernism is missing?

mushon commented 7 years ago

no matter what I do, it doesn’t appear on the map. Please instruct me what to write in the console exactly

yuvadm commented 7 years ago

@mushon it's one of the only debug prints in the console now (use updated version, it's much more tidy). In any case you can see it here

https://gist.github.com/yuvadm/93b7b2365b405d3585a7f299c76f38f9

and indeed Berlin/modernism is there

mushon commented 7 years ago

ok, I have a suspicion that the layer didn't really load from data but was still using the dataset I used in Mapbox. I fixed it by uploading the new places.json file you linked to Mapbox (it's much smaller than the one I used) and adding a hack to allow text overlap from zoom 3+. I would however suggest you test this by maybe trying to add an extra metropolis to Modernism that we know for sure doesn't exist in the Mapbox file (say Atlantis) and see if it is loaded or not. My bet is it isn't, which will be a problem. Almost there…

mushon commented 7 years ago

@yuvadm @talbenbasat

As I suspected back in October, the labels are not populated by the DB call but are coming from the json used in Mapbox instead (which includes Sydney for testing):

b43e5cdb-1e9b-46d9-8540-0d49b2271674

This issue will be resolved when Sydney is wiped off the map(!)