Skip to content
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

Refactor: Add types to legacy Redux modules #1856

Merged
merged 1 commit into from
Jan 24, 2020

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Jan 24, 2020

In this patch we're adopting the changes in #1855 and updating legacy
reducers, action types, and action creators. This serves both as a guide
for adding new Redux pieces while improving the overall type-safety and
completeness of the Redux subsystem in the app.

Testing

Except for the authentication status there should be no functional or
visual changes in this PR. Please test the affected flows anyway and
report if you find any discrepancies.

Because we're changing from using Symbols for authentication
status verify that you can login, logout, fail to login, etc…

@dmsnell dmsnell requested a review from a team January 24, 2020 00:13
@dmsnell dmsnell changed the base branch from types/full-redux-typing to develop January 24, 2020 18:13
In this patch we're adopting the changes in #1855 and updating legacy
reducers, action types, and action creators. This serves both as a guide
for adding new Redux pieces while improving the overall type-safety and
completeness of the Redux subsystem in the app.
@dmsnell dmsnell force-pushed the refactor/type-legacy-redux branch from d94eb82 to d9de6ac Compare January 24, 2020 18:16
Copy link
Contributor

@belcherj belcherj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, tested well.

@dmsnell
Copy link
Member Author

dmsnell commented Jan 24, 2020

Testing:

⭕️ set auto-hide menu bar - wasn't sure how to test
⭕️ had trouble unsetting Share analytics - in turning it off it would immediately turn back on automatically. this seems to only happen after seeing the get-note-titles issue below
⭕️ seeing Could not find note with id '…' from get-note-titles.ts when opening tag drawer
✅ Toggling markdown for a note changes the mode and toggles the preview icon
✅ Changing the list display mode in settings instantly updates the note list view
✅ Changing the sort type and direction instantly updates
⭕️ Alphabetical sort puts 'Something above Apple, lexically the quote appears before a
✅ Changing the theme instantly updated the display of the app
⭕️ There doesn't seem to be a way to go back to system setting for theme
✅ Using the search bar filters the notes as I expect
✅ Using the search bar with a tag selected also filters as expected
✅ Toggling the tag draw opens and closes it as expected

@dmsnell
Copy link
Member Author

dmsnell commented Jan 24, 2020

Failed to reproduce get-note-title bug in develop, switched back to this branch, unable to reproduce so I'm going to okay it as a glitch that we can look for.

Sort and theme issues exist in develop

@dmsnell dmsnell merged commit 3d24822 into develop Jan 24, 2020
@dmsnell dmsnell deleted the refactor/type-legacy-redux branch January 24, 2020 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants