[WIP] consolidate source text#3081
Conversation
2cbdf8a to
11e4542
Compare
Codecov Report
@@ Coverage Diff @@
## master #3081 +/- ##
==========================================
+ Coverage 66.15% 66.27% +0.11%
==========================================
Files 74 74
Lines 2603 2612 +9
Branches 527 528 +1
==========================================
+ Hits 1722 1731 +9
Misses 881 881
Continue to review full report at Codecov.
|
| // to rethink how we type async actions. | ||
| function updateText(state, action: any): Record<SourcesState> { | ||
| const source = action.source; | ||
| const text = getTextFromAction(action); |
There was a problem hiding this comment.
this can be more than text. for example it can also have an id, content type, and loading state. maybe we can name this a bit better? sourceTextProps comes to mind, or just updateProps
| // "start" and "error" states but we don't type it like that. We need | ||
| // to rethink how we type async actions. | ||
| function _updateText(state, action: any): Record<SourcesState> { | ||
| function getTextFromAction(action: any) { |
There was a problem hiding this comment.
this returns more than text, maybe getTextPropsFromAction is clearer?
| contentType: sourceText.contentType | ||
| }) | ||
| ); | ||
| return state.mergeIn(["sources", source.id], source); |
There was a problem hiding this comment.
maybe we can have a comment here about why we are merging, rather than setting? I saw above there was such a comment in the previous version of the code, might be helpful here too
There was a problem hiding this comment.
I think the comment before covered a special case that we no longer are concerned with. I am not sure if I have something that I want to say here except that we're adding source fields :)
| } | ||
|
|
||
| const hasLoaded = sourceText && !sourceText.get("loading"); | ||
| const hasLoaded = selectedSource && !selectedSource.get("loading"); |
11e4542 to
6414f0f
Compare
Associated Issue: #3078
Summary of Changes
I started refactoring the sources store to remove SourceText. The reason is that it is not needed and creates extra complexity around selectors and typing in our components, actions, and utilities.