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

Remove all usage of redux-ui in favor React Redux/Redux Saga #857

Closed
schnogz opened this issue Aug 9, 2018 · 6 comments
Closed

Remove all usage of redux-ui in favor React Redux/Redux Saga #857

schnogz opened this issue Aug 9, 2018 · 6 comments
Labels
good first issue An easier issue good for first time contributors help wanted Help is desired by developer(s) tech debt Work identified as tech debt

Comments

@schnogz
Copy link
Contributor

schnogz commented Aug 9, 2018

🐛 Bug Report (Tech Debt) 🐛

Description

When the codebase first started out, redux-ui was used in a few spots for quick state management. Now that we have defined consistent patterns for state management via React Redux & Redux Saga, let's remove redux-ui completely in favor of the latter.

Expected Outcomes

All usages of redux-ui are eventually removed. This can be done all at once or incrementally, whichever works best.

Additional Information

redux-ui Docs: https://github.com/tonyhb/redux-ui
React Redux Docs: https://github.com/reduxjs/react-redux
Redux Saga Docs: https://github.com/redux-saga/redux-saga

@schnogz schnogz added enhancement An enhancement for an existing feature help wanted Help is desired by developer(s) good first issue An easier issue good for first time contributors tech debt Work identified as tech debt and removed enhancement An enhancement for an existing feature labels Aug 9, 2018
@mchlcrtz
Copy link
Contributor

@schnogz I would like to get on this when I can! It looks like they are mostly in the container files. Would you have a preference on where to start?

@schnogz
Copy link
Contributor Author

schnogz commented Sep 4, 2018

@mchlcrtz that would be great. Some of the easiest spots would be in the Security Center. blockchain-wallet-v4-frontend/src/scenes/SecurityCenter/~. Feel free to open PRs whenever it makes sense and as needed

@mchlcrtz
Copy link
Contributor

mchlcrtz commented Sep 8, 2018

Hi @schnogz, should I be adding my own sagas in blockchain-wallet-v4-frontend/src/data? If so, can you describe the design of data flow in that directory? Thank you!

Also, with simple ui state changes such as toggling, would it be okay to handle them with normal actions/reducers?

@schnogz
Copy link
Contributor Author

schnogz commented Sep 10, 2018

I think most code that uses redux-ui is pretty simple stuff such as toggle states and what not. You can just use the standard react component state to handle all this. Here is an example:
https://github.com/blockchain/blockchain-wallet-v4-frontend/blob/9a9a5cdc9d52b244d71b1b3197fd2a7b81d78b39/packages/blockchain-info-components/src/Dropdowns/SimpleDropdown/index.js

Edit: I see that you have already made a PR for some of this using redux. I will take a look later today but we probably want to redo it to use just react states. sorry about that man.

@schnogz schnogz mentioned this issue Sep 13, 2018
4 tasks
@plondon
Copy link
Contributor

plondon commented Sep 20, 2018

Can we close this?

@schnogz schnogz mentioned this issue Dec 4, 2018
4 tasks
@plondon
Copy link
Contributor

plondon commented Jan 15, 2019

Closing!

@plondon plondon closed this as completed Jan 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue An easier issue good for first time contributors help wanted Help is desired by developer(s) tech debt Work identified as tech debt
Projects
None yet
Development

No branches or pull requests

3 participants