-
Notifications
You must be signed in to change notification settings - Fork 15
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
Suse Manager Settings - Frontend refactoring #2819
Conversation
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.
I can't find anything wrong with this PR 😄
Just for future viewers, we decided to have custom hooks in this page but it should really be taken with a grain of salt and we still don't want to adopt them in the rest of the app for now
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.
Hey nice great work man, just one typo 👍
const [fetchError, setFetchError] = useState(false); | ||
const [testingSettings, setTestingSettings] = useState(false); | ||
|
||
const fetchSuseManagerSettings = async () => { |
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 is very clean, i like it :D
]); | ||
}); | ||
|
||
it('should test the suse manager settings and spawn a toast when the connection succeded', async () => { |
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.
typo in succeded --> succeeded 👍
Description
This PR removes from redux state/sagas the suse manager settings.
This pr creates a custom hook to handle the lifecycle of suse manager settings.
UI/UX Interactions not changed.
How was this tested?
Automated tests