Refactor: Move revision-fetching into new Simperium middleware #1921
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.
As part of our efforts to replace the "wonky app state" we have come across
a few different elements of state that are closer matches to middleware behaviors
than reducer behaviors. Fetching the revisions for a note is one such activity.
Revisions should be gathered whenever we open the revision slider. Previously
when opening the slider we were making two dispatches: one to set the slider
visibility and another to fetch the revisions. Two dispatches are not necessary
and the fetching is purely as side-effect of opening the panel.
In this patch we're creating a new middleware file where we plan to move all
Simperium side-effects eventually. Right now it only watches for appropriate
changes to the revision slider visibility.
Note that the use of middleware has removed the need to chain dispatch calls
as was previously done between
noteRevisions
andnoteRevisionsLoaded
. Wenow have a reactive response to
REVISIONS_TOGGLE
and a more declarativeSTORE_REVISIONS
action.Because revision state is so closely related to this work we are also moving
the revision state into Redux proper and fixing a bug or two in the process.
Previously the revision slider would stay open when selecting another
note or when creating a new note through the keyboard shortcuts. It did
this because the React component was imperatively calling to close the
panel on
outsideClick
. Now it properly closes when using the keyboardshortcuts because the
showRevisions
reducer is now listening for thoseactions. We could have left it open but I made the decision that it was
best to prevent confusion to close it. In
develop
, even though theslider stays open it fails to fetch the revisions when changing notes.
This too is due to the existing imperative and confusing data flows.
Previously it was possible to select another note in the time it took
to download the note revisions. In this race you could potentially see
the revisions for another note. Now this has been resolved by only
storing the note revisions if we are still on the same note as when
they were requested.
Testing
Use the checklist but add extra tests for revisions.
To see the existing bug in production open the revision slide for a note and
then navigate the note list with ⌘+⇧+j or the equivalent in non-macOS
systems. Similarly you can navigate in the opposite direction with k instead
of j. Note that the revision slider stays open and also that it doesn't
fetch the revisions of the newly-selected note. Repeat the experiment by creating
a new note with ⌘+n.