Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Impact Layers & Refactor App Redux Structure #55

Merged
merged 22 commits into from
May 26, 2020

Conversation

bencpeters
Copy link
Collaborator

@bencpeters bencpeters commented May 22, 2020

Add Impact Layers & Refactor App Redux Structure

This PR adds Impact Layers.

It also significantly refactors the layer configuration interface for much better type safety, decent (but not 100% perfect) parsing of the config file for ensuring proper config definitions.

It removes Immutable.js state, since redux toolbox is already using Immer.js for state immutability under the hood, and they weren't playing well together.

Refactors where state lives by adding concept of layer-specific data in Redux along with async loaders.

Adds loading animation.
Remaining issues that would be nice to have in the near future:

  • Impact layers aren't linked up with the calendar yet
  • PR removed async initial state - we should grab the calendar capabilities in MapView by creating an async thunk to load that data.
  • Display for loading errors (e.g. a Snackbar component) - right now they're saved in Redux, but not displayed.
  • Improve performance of Impact algorithm for faster loading (I have some ideas on how to do this).

@bencpeters bencpeters requested a review from ericboucher May 22, 2020 23:49
@ericboucher
Copy link
Collaborator

So this actually breaks the calendar for climate layers as well, correct?
Let's make sure we solve 1 & 2 before merging

Copy link
Collaborator

@ericboucher ericboucher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. A few comments:

  • Layer toggle is now very extremely slow, not just for impact layers.

Screen Shot 2020-05-25 at 12 17 37 PM

Any ideas on what might be happening?

  • I also like the layer structure change, but it also means we need to take care of Limit to one "has_date" layer at a time #28 very soon.

  • Often times, no admin boundaries are above the threshold which makes the user unsure if the impact calculations actually ran or not. Can we color boundaries below threshold in a light grey maybe? Thoughts?

// GDAL style extent: xmin ymin xmax ymax
export type Extent = [number, number, number, number];

// Placeholder for Geotiff image (since library doesn't contain types)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be in type or customs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be either, really. In this case I put them here because I felt like the library was going to be basically contained to this file. But especially with a library that is used across the app, we'd probably want them to go in custom

This was linked to issues May 26, 2020
@bencpeters
Copy link
Collaborator Author

* Layer toggle is now very extremely slow, not just for impact layers.

This is happening after we load a lot of data in Redux state (e.g. load an impact layer - you'll notice it doesn't happen if you view other layers before an impact layer, just after). It's being caused by the immutable middleware that the Redux toolkit has loaded in development mode which does a deep object compare of the entire state tree to make sure that there are no immutable violations (e.g. no reducer and no other client of the data has modified it during the redux lifecycle). When the state object gets really large (because it has all the raster data loaded in), this deep object checking takes a long time. This middleware is disabled in production though, so it shouldn't happen in prod.

That being said, it is kinda annoying in development, I might look into whether or not we can easily get rid of it. An easy fix would be to just remove the loaded data from state as soon as the layer goes out of scope, but it's kinda nice to save the data around in a persistent cache so that the load time is reduced if the user goes back to this layer. The flip side of that though is that it could result in a very high memory footprint for the app as a whole if the user ends up viewing a lot of different impact layers (or a lot of different dates for an impact layer) over the course of a session...

@bencpeters bencpeters merged commit 69deaec into master May 26, 2020
@bencpeters bencpeters deleted the add-impact-layers-refactor-redux branch May 26, 2020 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Load WCS data Display the number of people exposed to a hazard
2 participants