search

worldbank-cdrp / historic-risk-explorer

A historic disaster risk explorer
Other
1 stars 1 forks source link

Maps #94

Closed danielfdsilva closed 6 years ago

danielfdsilva commented 6 years ago

Addresses #86 #87 #88 #89 #92

danielfdsilva commented 6 years ago

@Rub21 To solve some of the issues with the map I ended up doing an big refactor. This is now implemented more in line with the react way (within the realm of what's possible for this project). I didn't mean to go over your work, but this made it really easy to add popups so I just went ahead and included them.

@ascalamogna The markup and style for the tooltip is likely to need a review. Go at it: https://github.com/worldbank-cdrp/historic-risk-explorer/blob/204c4b5733b76c8b28aa05f3810373b81b7d1334/app/assets/scripts/components/AnalysisMap.js#L327-L338

olafveerman commented 6 years ago

Thanks a bunch @danielfdsilva ! Had a look and it seems to address the issues you referenced.

@ascalamogna want to give a final and see if it addresses the issues you were dealing with?

ascalamogna commented 6 years ago

@danielfdsilva I styled the tooltip but I've noticed that if you have the level on gridded and turn off the footprint, the whole base map disappears. screen shot 2018-03-13 at 11 53 41 am

danielfdsilva commented 6 years ago

@ascalamogna Is this a consistent behavior? Are there any errors in the console? I've tried both on Chrome and Firefox and can't replicate.

ascalamogna commented 6 years ago

@danielfdsilva these are the errors I'm getting in the console. It happens only if you have the admin level on and then toggle off the footprint. I'm in Chrome.

screen shot 2018-03-14 at 8 54 15 am
danielfdsilva commented 6 years ago

@ascalamogna That is an unrelated error. I tried exactly as you said. Admin level on gridded and toggling the footprint, but works well. Also tried all the other options while toggling, and on a different computer.

Can you try on a different machine or your side as well?

olafveerman commented 6 years ago

Reviewed with @ascalamogna . All looking good, merging.