-
Notifications
You must be signed in to change notification settings - Fork 34
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
Conversation
…compile-time safety, and no dangerous assertions
…r immutability). Bumped version of redux-toolkit, added async layer data loader handler
So this actually breaks the calendar for climate layers as well, correct? |
There was a problem hiding this 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.
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) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 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... |
…/prism-frontend into geotiff-playground
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: