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.

Sign up for GitHub

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

Jump to bottom

Move Search state to redux #1881

Merged
merged 2 commits into from Feb 13, 2020
Merged

Move Search state to redux #1881

merged 2 commits into from Feb 13, 2020

Conversation

belcherj
Copy link
Contributor

@belcherj belcherj commented Feb 5, 2020 •
edited

Fix

This moves the search state to Redux from flux.

  • Removed ref prop that wasn't being used
  • Renamed filter to searchQuery which better describes what it is and increases the ability to find instances of where it is used in the future.
  • Removed props from components that only pass them through

Test

  1. Tests search very heavily.

@belcherj belcherj requested a review from a team February 5, 2020 15:14
@belcherj belcherj self-assigned this Feb 5, 2020
@belcherj belcherj marked this pull request as ready for review February 5, 2020 15:25
dmsnell
dmsnell reviewed Feb 6, 2020
View reviewed changes
lib/state/ui/middleware.ts Outdated Show resolved Hide resolved
dmsnell
dmsnell reviewed Feb 6, 2020
View reviewed changes
lib/note-list/index.tsx Outdated Show resolved Hide resolved
dmsnell
dmsnell requested changes Feb 6, 2020
View reviewed changes
Copy link
Contributor

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

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

Looks like we do need to rebase against develop to pull in changes from #1880 and then test this. I didn't test yet because of the rebase/update. I didn't want to test and catch the same bugs the other PRs have addressed

@belcherj belcherj force-pushed the refactor/move-search-to-redux branch from b1459ae to f1fd6dd Compare February 10, 2020 20:52
@codebykat codebykat mentioned this pull request Feb 12, 2020
Merged
@belcherj belcherj added the state label Feb 12, 2020
@belcherj belcherj force-pushed the refactor/move-search-to-redux branch 3 times, most recently from 32a550f to 12e17ae Compare February 13, 2020 19:21
@belcherj belcherj force-pushed the refactor/move-search-to-redux branch from 12e17ae to fc8125f Compare February 13, 2020 20:22
@belcherj belcherj merged commit d28c963 into develop Feb 13, 2020
@belcherj belcherj deleted the refactor/move-search-to-redux branch February 13, 2020 20:37
@dmsnell dmsnell mentioned this pull request Feb 19, 2020
Closed
41 tasks
@codebykat codebykat added this to the state refactor milestone Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmsnell dmsnell dmsnell requested changes

Assignees

@belcherj belcherj

Labels
None yet
Projects
None yet
Milestone
state refactor
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@belcherj @dmsnell @codebykat