-
Notifications
You must be signed in to change notification settings - Fork 511
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
Fixed : Disable Submit Button #9526
Changes from 2 commits
f66ab92
3438758
2557354
7d7b8f4
b46c85d
6a8092f
96903ce
2c88392
9193e17
cf2bd9e
aeb32d5
8e179fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -70,6 +70,7 @@ | |||||
"@sentry/browser": "^8.45.1", | ||||||
"@tanstack/react-query": "^5.62.3", | ||||||
"@tanstack/react-query-devtools": "^5.62.7", | ||||||
"@types/lodash": "^4.17.13", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix incorrect @types/lodash version The version Update the version to the latest stable: - "@types/lodash": "^4.17.13",
+ "@types/lodash": "^4.14.202", 📝 Committable suggestion
Suggested change
|
||||||
"@yudiel/react-qr-scanner": "^2.0.8", | ||||||
"bowser": "^2.11.0", | ||||||
"browser-image-compression": "^2.0.2", | ||||||
|
@@ -88,6 +89,7 @@ | |||||
"i18next": "^23.16.4", | ||||||
"i18next-browser-languagedetector": "^8.0.2", | ||||||
"i18next-http-backend": "^3.0.1", | ||||||
"lodash": "^4.17.21", | ||||||
"postcss-loader": "^8.1.1", | ||||||
"qrcode.react": "^4.1.0", | ||||||
"raviger": "^4.1.2", | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets not modify the old custom Shadcn's form is built to be used with shadcn's form fields, not our form field components. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import isEqual from "lodash/isEqual"; | ||
import { useEffect, useMemo, useRef, useState } from "react"; | ||
|
||
import { Cancel, Submit } from "@/components/Common/ButtonV2"; | ||
|
@@ -47,6 +48,7 @@ const Form = <T extends FormDetails>({ | |
const [isLoading, setIsLoading] = useState(!!asyncGetDefaults); | ||
const [state, dispatch] = useAutoSaveReducer<T>(formReducer, initial); | ||
const formVals = useRef(props.defaults); | ||
const [isFormModified, setIsFormModified] = useState(false); | ||
|
||
useEffect(() => { | ||
if (!asyncGetDefaults) return; | ||
|
@@ -82,12 +84,14 @@ const Form = <T extends FormDetails>({ | |
}); | ||
} else if (props.resetFormValsOnSubmit) { | ||
dispatch({ type: "set_form", form: formVals.current }); | ||
setIsFormModified(false); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling for form submission failures The form modified state is reset only on successful submission. Consider handling submission failures to maintain consistent state. } else if (props.resetFormValsOnSubmit) {
dispatch({ type: "set_form", form: formVals.current });
setIsFormModified(false);
+} else {
+ // Keep isFormModified true if submission fails
+ setIsFormModified(true);
} Also applies to: 93-95 |
||
}; | ||
|
||
const handleCancel = () => { | ||
if (props.resetFormValsOnCancel) { | ||
dispatch({ type: "set_form", form: formVals.current }); | ||
setIsFormModified(false); | ||
} | ||
props.onCancel?.(); | ||
}; | ||
|
@@ -123,13 +127,19 @@ const Form = <T extends FormDetails>({ | |
return { | ||
name, | ||
id: name, | ||
onChange: ({ name, value }: FieldChangeEvent<T[keyof T]>) => | ||
onChange: ({ name, value }: FieldChangeEvent<T[keyof T]>) => { | ||
const newForm = { | ||
...state.form, | ||
[name]: value, | ||
}; | ||
setIsFormModified(!isEqual(newForm, formVals.current)); | ||
dispatch({ | ||
type: "set_field", | ||
name, | ||
value, | ||
error: validate?.(value), | ||
}), | ||
}); | ||
}, | ||
value: state.form[name], | ||
error: state.errors[name], | ||
disabled, | ||
|
@@ -149,7 +159,7 @@ const Form = <T extends FormDetails>({ | |
<Submit | ||
data-testid="submit-button" | ||
type="submit" | ||
disabled={disabled} | ||
disabled={disabled || !isFormModified} | ||
label={props.submitLabel ?? "Submit"} | ||
/> | ||
</div> | ||
|
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.
Shouldn't be in the commit.
How are you committing your merge changes? Cli or GUI 🤔
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.
CLI