search

CMU-CREATE-Lab / data-visualization-tools

EarthTime, and various data visualization libraries
Other
14 stars 5 forks source link

Sea Level Rise Layers Missing Legends #165

Closed jjkohler closed 3 years ago

jjkohler commented 3 years ago

https://staging.earthtime.org/explore#waypoints=1heLmeuPp7j4itr0cK8H4chugOpp7cU8p_VEB9CFfPlY.0&v=21.55167,99.68213,2.714,latLng&t=0.62&ps=50&l=blsat,slr4&bt=0.03&et=0.83&startDwell=0&endDwell=0 Expected: image

https://staging.earthtime.org/explore#waypoints=1heLmeuPp7j4itr0cK8H4chugOpp7cU8p_VEB9CFfPlY.0&v=28.01631,-86.9635,4.99,latLng&t=1.04&ps=50&l=bdrk,sea_level_rise_10m_v2&bt=0.03&et=2.03&startDwell=0&endDwell=0 Expected: image

gabrielo commented 3 years ago

@jjkohler This should be resolved with commits 3cbd55cc8b3593f63197753ca5ba76f3332bb139 and be700b25029a775ba051fffff16b1cbd8a5cbbdd

jjkohler commented 3 years ago

Looking good! One very small issue that seems to have emerged with this fix: when any of this type of layer (e.g. 4.0, 2.0, etc.) are paused and then turned on/off or switched between, the legend changes and notably the "Multi-century sea level increase: ... " line is missing from the legend, until playback is restarted - either with the play button or by manually scrubbing through the timeline.

Example of how legend looks when switching to paused layer: image

Vs.

How it previously looked in this case in Golden: image

gabrielo commented 3 years ago

@jjkohler paused layer should be resolved with commit ac515fe2a961417fee1ff53b5f03af00e52c2e58

pdille commented 3 years ago

@gabrielo Thanks, Gabriel! To add to this, I just pushed a commit (262164320c541) to the layer toggle flow that allows us to run code when a layer is turned on/off. I've modified your code here to take advantage of that, so this way we aren't modifying the DOM for the sea level rise legend with each draw. It's only updated when it needs to, like your initial commit. Likely minor perf improvement here, but I think having this hook into the layer toggling will be useful in the future for other layers similar to seal level rise that may also need specific prep/cleanup.

@jjkohler Lastly, I added the mutually exclusive flag to these 4 sea level rise layers so you can't turn them all on at once. Note that the 10m/20m specific sea level rise layers do not yet have this flag, since they are part of the davos2019 spreadsheet and don't yet know about this new spreadsheet column.

jjkohler commented 3 years ago

This all looks to be solved.