Skip to content

fix: TextField validateOnChange with defaultValue executed on start #2411

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 10, 2023
Merged

fix: TextField validateOnChange with defaultValue executed on start #2411

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2eaf9b
fix: TextField validateOnChange on web
AmitShwarts Jan 9, 2023
937d501
code review
AmitShwarts Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"docs:build": "node scripts/buildDocs.js",
"snippets:build": "node scripts/generateSnippets.js",
"demo": "./scripts/demo.sh",
"release": "node ./scripts/release.js"
"release": "node ./scripts/release.js",
"start:web": "npm --prefix webDemo run start"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please move it to be after the start script? just to keep it in a logical order :)

},
"dependencies": {
"babel-plugin-transform-inline-environment-variables": "^0.0.2",
Expand Down
6 changes: 3 additions & 3 deletions src/incubator/TextField/useFieldState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ export default function useFieldState({
if (Constants.isWeb && !props.value && props.defaultValue && props.defaultValue !== value) {
setValue(props.defaultValue);

if (validateOnChange) {
if (validateOnStart) {
validateField(props.defaultValue);
}
}

/* On purpose listen only to props.defaultValue change */
/* eslint-disable-next-line react-hooks/exhaustive-deps*/
}, [props.defaultValue, validateOnChange]);
}, [props.defaultValue, validateOnStart]);

useEffect(() => {
if (validateOnStart) {
Expand All @@ -42,7 +42,7 @@ export default function useFieldState({
if (props.value !== value) {
setValue(props.value);

if (validateOnChange) {
if (validateOnChange && (!Constants.isWeb || value !== props.defaultValue)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I've seen, it also reproduces on mobile
Please verify it and if it's true, add this condition constantly :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep! you are right :)
i just pushed a fix for your comments.

validateField(props.value);
}
}
Expand Down
9 changes: 8 additions & 1 deletion webDemo/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,14 @@ const itemsToRender: ItemToRender[] = [
migrate
defaultValue={defaultValue}
containerStyle={{marginBottom: 10}}
placeholder="type here..."
placeholder="Enter your email..."
validationMessage={['Email is required', 'Email is invalid']}
validationMessagePosition={'top'}
enableErrors
validate={['required', 'email']}
validateOnStart={false}
validateOnChange
validateOnBlur={false}
onChangeText={(text: string) => {
console.log(text);
}}
Expand Down