Skip to content

Fix: Form validation runs on blur in New task details page #24011

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 6 commits into from
Aug 7, 2023
Merged

Fix: Form validation runs on blur in New task details page #24011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f7bab13
fix: form validation runs on blur
tienifr Aug 1, 2023
c16ef56
fix lint
tienifr Aug 1, 2023
dde8208
refocus text input in Chrome only
tienifr Aug 1, 2023
671ca30
fix lint
tienifr Aug 1, 2023
81010c0
fix lint
tienifr Aug 1, 2023
4897c4d
Merge branch 'main' into fix/23552
tienifr Aug 4, 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
28 changes: 16 additions & 12 deletions src/components/Form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import ScrollViewWithContext from './ScrollViewWithContext';
import stylePropTypes from '../styles/stylePropTypes';
import {withNetwork} from './OnyxProvider';
import networkPropTypes from './networkPropTypes';
import Visibility from '../libs/Visibility';

const propTypes = {
/** A unique Onyx key identifying the form */
Expand Down Expand Up @@ -304,18 +305,21 @@ function Form(props) {
defaultValue: undefined,
errorText: errors[inputID] || fieldErrorMessage,
onBlur: (event) => {
// We delay the validation in order to prevent Checkbox loss of focus when
// the user are focusing a TextInput and proceeds to toggle a CheckBox in
// web and mobile web platforms.
setTimeout(() => {
setTouchedInput(inputID);

// To prevent server errors from being cleared inadvertently, we only run validation on blur if any form values have changed since the last validation/submit
const shouldValidate = !_.isEqual(inputValues, lastValidatedValues.current);
if (shouldValidate) {
onValidate(inputValues);
}
}, 200);
// Only run validation when user proactively blurs the input.
if (Visibility.isVisible() && Visibility.hasFocus()) {
Copy link
Member

Choose a reason for hiding this comment

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

Always prefer early returns.

Copy link
Contributor Author

@tienifr tienifr Aug 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks but we still have other codes below which are not relating to the check.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe move the other code above this if block if there are no issue with it and then use return.

// We delay the validation in order to prevent Checkbox loss of focus when
// the user are focusing a TextInput and proceeds to toggle a CheckBox in
// web and mobile web platforms.
setTimeout(() => {
setTouchedInput(inputID);

// To prevent server errors from being cleared inadvertently, we only run validation on blur if any form values have changed since the last validation/submit
const shouldValidate = !_.isEqual(inputValues, lastValidatedValues.current);
if (shouldValidate) {
onValidate(inputValues);
}
}, 200);
}

if (_.isFunction(child.props.onBlur)) {
child.props.onBlur(event);
Expand Down
3 changes: 2 additions & 1 deletion src/components/TextInput/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import BaseTextInput from './BaseTextInput';
import * as baseTextInputPropTypes from './baseTextInputPropTypes';
import DomUtils from '../../libs/DomUtils';
import Visibility from '../../libs/Visibility';
import * as Browser from '../../libs/Browser';

function TextInput(props) {
const textInputRef = useRef(null);
Expand All @@ -21,7 +22,7 @@ function TextInput(props) {
}

removeVisibilityListenerRef.current = Visibility.onVisibilityChange(() => {
if (!Visibility.isVisible() || !textInputRef.current || DomUtils.getActiveElement() !== textInputRef.current) {
if (!Browser.isMobileChrome() || !Visibility.isVisible() || !textInputRef.current || DomUtils.getActiveElement() !== textInputRef.current) {
return;
}
textInputRef.current.blur();
Expand Down