-
Notifications
You must be signed in to change notification settings - Fork 136
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
Replace connect
with hooks in 3 components
#7552
Replace connect
with hooks in 3 components
#7552
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
members: "Team Members", | ||
api: "API Keys", | ||
}; | ||
setTab(views[view]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was trying to derive the local state from the selected state - so I've just refactored this to use the actual selected state right from the start
@@ -130,7 +129,6 @@ const settings: Settings< | |||
settings?: any; | |||
isAdmin: boolean; | |||
workspaceId: string; | |||
hideModal: PropsFromRedux["hideModal"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I read this correctly... only TeamMembers
were actually using hideModal
so I've just moved it directly to it and cleaned this up from the parent components
Thanks! I'll merge this probably early next week |
ffdbebe
to
70a58a3
Compare
I was playing with reduxjs/react-redux#1940 and I've started looking into the codebase here. Since I've never actually used "modern Redux" in an app I've figured out that it would be a nice exercise to migrate a few components to hooks.
Feel free to do whatever u want with this PR 😉 cc @markerikson