-
Notifications
You must be signed in to change notification settings - Fork 702
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
Fix install repo firing twice (#3678) #3891
Conversation
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
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.
Thanks for fixing that issue! Great work :)
@@ -620,7 +627,7 @@ export function AppRepoForm(props: IAppRepoFormProps) { | |||
</Alert> | |||
)} | |||
<div className="margin-t-xl"> | |||
<CdsButton disabled={validating} onClick={install}> | |||
<CdsButton type="submit" disabled={validating}> |
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.
Why are you removing the onClick event? Perhaps this same function is also being triggered in the onSubmit?
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.
U nailed it! install
function was being called twice: here and inside form's onSubmit logic.
@@ -49,6 +49,7 @@ const TYPE_OCI = "oci"; | |||
|
|||
export function AppRepoForm(props: IAppRepoFormProps) { | |||
const { onSubmit, onAfterInstall, namespace, kubeappsNamespace, repo, secret } = props; | |||
const workingRef = useRef(false); |
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'd maybe choose another name, a more self-explanatory one like isInstallingRef
?
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.
You are right, will rename it. Initially I wanted to make the solution something general for all forms+buttons :)
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Signed-off-by: Rafa Castelblanque rcastelblanq@vmware.com
Description of the change
Fixes the problem of "Install Repo" button being fired multiple times.
Benefits
Repo installation process is not triggered more than once.
Possible drawbacks
N/A
Applicable issues
Additional information
Makes use of React's useRef