-
Notifications
You must be signed in to change notification settings - Fork 2.4k
🔨 Refactored, 🧠 Overmind, Hacktober | Refactor StorageManagementModal to use overmind #2753
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
Conversation
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/codesandbox/codesandbox-client/iieszkqhi |
Saeris
left a comment
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.
Looks great! Just update the imports as requested and we can merge this!
| } from './elements'; | ||
| import FilesList from './FilesList'; | ||
|
|
||
| const StorageManagementModal = () => { |
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.
| const StorageManagementModal = () => { | |
| export const StorageManagementModal: React.FC = () => { |
We'd like to move everything to named exports instead, so let's change this line.
| ); | ||
| }; | ||
|
|
||
| export default StorageManagementModal; |
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.
| export default StorageManagementModal; |
And we can remove this one.
Last thing I'd need you to do for me is to search the project for imports of StorageManagementModal and update those lines to used named imports instead. Should only be a single line for a modal like this.
|
Duplicate of #2808 |
🔨 Refactored, 🧠 Overmind, Hacktober | Refactor StorageManagementModal to use overmind
What kind of change does this PR introduce?
Refactors code as a part of hacktoberfest mentioned in #2621.
@Saeris @christianalfoni @SaraVieira
What is the current behavior?
/app/pages/common/Modals/StorageManagementModal/index.jswas implemented using a class component without having any state and life cycle methods and also usedinjectandhookObserverfromapp/componentConnectorsWhat is the new behavior?
Refactored StorageManagementModal to FC and replaced
inject,hookObserverwithuseOvermindWhat steps did you take to test this? This is required before we can merge.
StorageManagementModalfromUserMenuyarn lintyarn testChecklist