start reduxing activity state #426
Conversation
Codecov Report
@@ Coverage Diff @@
## master #426 +/- ##
==========================================
- Coverage 81.47% 78.87% -2.61%
==========================================
Files 120 127 +7
Lines 1636 1728 +92
Branches 136 147 +11
==========================================
+ Hits 1333 1363 +30
- Misses 273 331 +58
- Partials 30 34 +4
Continue to review full report at Codecov.
|
brendansudol
changed the title
| }; | ||
|
|
||
| const mapStateToProps = ({ activities: { byId } }, { aId, num }) => { | ||
| const activity = byId[aId]; |
There was a problem hiding this comment.
I'm curious why you get the activity from redux store by ID rather than taking it as a param?
There was a problem hiding this comment.
yeah, i wasn't really sold on this either; i read that it was better performance to pass down the id and look up the data object via connect, let me track down the article
There was a problem hiding this comment.
There was a problem hiding this comment.
Fascinating. Really gets into the weeds of how React handles re-rendering as state changes.
| toolbar={EDITOR_CONFIG} | ||
| editorState={descLong} | ||
| onEditorStateChange={this.onEditorChange('descLong')} | ||
| onBlur={this.syncEditorState('descLong')} |
There was a problem hiding this comment.
To make sure I'm clear on this: as the editor changes, update the component state so the editor stays editable; when the editor blurs, grab the HTML and sync that to redux state?
| const reducer = (state = initialState, action) => { | ||
| switch (action.type) { | ||
| case ADD_ACTIVITY: { | ||
| const id = Math.max(...state.allIds, 0) + 1; |
There was a problem hiding this comment.
Writing this out for my own benefit... Creating an ID here is safe because when we sync to the API, we can refresh the redux state to match the "real" ID and that will cascade through the app and everything will be okay. 🤔
I'm not sure that's true, but let's fix it when it happens rather than spend any time theorizing about what might happen in the future since it can't possibly happen right now.
There was a problem hiding this comment.
yep, that was my thought process as well
| altApproach: '' | ||
| }); | ||
|
|
||
| const updateEntry = (state, action) => { |
There was a problem hiding this comment.
Do you think using updeep here might be good? The code would look something like...
return updeep({ byId: { [id]: { [name]: value }}}, state);
There was a problem hiding this comment.
whoa - never heard of it, but i'm intrigued!!
There was a problem hiding this comment.
updeep is awesome, gonna switch to this going forward -- great tip!
Overview: Ok, this is the start of wiring up our new post-pivot activity stuff to redux. I'm leaving the former
pocpage as is (it's now at the root page) so we have it as a reference as we're doing this (and people can still look and click on stuff). and the new stuff is at/activities. Eventually, this will look very similar / better than what's at thepocpage, AND it will sync with our client data store, which we can then easily push to the backend.More details:
In terms of the structure of the
activitiesstate, I'm following this normalized structure (https://redux.js.org/recipes/structuring-reducers/normalizing-state-shape) which will help with performance.I don't yet love the structure / API for the UPDATE_ACTIVITY action right now, but it's a start and it works.
For the rich editor fields, we convert the
EditorStateobject to html when syncing with our state (and from html to thatEditorStateon initialization); we can abstract/consolidate these conversions as we build out additional components that need this.When reviewing, please go to
/activitiespage and play around with the fields and inspect the redux logger console statements, too.The next steps are adding more components within the
ActivityDetailAll(similar to what I've done withActivityDetailDescription).I also improved input focus states and made them consistent across our various input types (input, textarea, editor)
This pull request is ready to merge when...
The experience passes a basic manual accessibility audit (keyboard nav, screenreader, text scaling) OR an exemption is documentedThis feature is done when...
Design has approved the experienceProduct has approved the experience