Feature/map routing #3508
Merged
Feature/map routing #3508
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pjosh
approved these changes
Jul 5, 2018
j8seangel
reviewed
Jul 5, 2018
| @@ -1,74 +0,0 @@ | |||
| export const initialState = { | |||
| import { getMapSettings } from '../../map-selectors'; | ||
|
|
||
| const mapStateToProps = ({ location }) => ({ | ||
| settings: getMapSettings(location) |
There was a problem hiding this comment.
I'm not sure this working perfectly, we have to take a look here https://github.com/reduxjs/reselect#sharing-selectors-with-props-across-multiple-component-instances to be 100% sure.
| @@ -44,8 +50,8 @@ class MapContainer extends PureComponent { | |||
| } | |||
|
|
|||
| componentWillReceiveProps(nextProps) { | |||
There was a problem hiding this comment.
This will be deprecated soon, why don't use componentDidUpdate instead?
|
Loving the structure selector! Feel free to merge and thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This is a big one. Both in file changes and architecture, so please review with an open mind and with caution!
So the new map we are going to need to manage a large amount of state specific to many components on the page. Additionally this map needs to be able to exist in the map page, dashboards, and potentially other forms and pages. So how do we make sure we can manage the map state robustly, in isolation, and not have to faff around with lots of url and location functions every time we use the map. Simple! We use the same encoding method we have been using for the widgets, but with some updates:
?map=statekey in the url and bind it to the url by creating a singlesetMapSettingsaction to dispatch to redux-first-router.querySerializerof the routerquerySerializerencoder to make sure what we are dealing with is an object and then encode it and send to the urllocationkey on the store is now holding the state of each component under a suitable key which can easily be passed to the component with its mapStateToProps and selectors.This approach removes the need to dispatch an additional action to sync the url with the store like before, improving performance, and reduces complexity in the router when wanting to include a new component in a page.
Disadvantage: unreadble urls.
NOTE: I have left in some commented functions incase this approach is not approved to that we can comprise on some features.
Demo
Open up redux tools, and try the zoom buttons on the map.
Credit: pretty much @j8seangel's PR.