Skip to content
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 form submit behavior #34471

Merged
merged 28 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
fe878dc
fix submit behavior on native devices
shubham1206agra Jan 13, 2024
36301d1
remove unused logic
shubham1206agra Jan 13, 2024
7f8e548
small bug fix
shubham1206agra Jan 13, 2024
8d51fa2
make submitOnEnter use consistent
shubham1206agra Jan 16, 2024
3854a9a
fix prettier
shubham1206agra Jan 16, 2024
abd18c6
Merge branch 'Expensify:main' into fix-form-submit
shubham1206agra Jan 16, 2024
c3fc139
Merge branch 'Expensify:main' into fix-form-submit
shubham1206agra Jan 17, 2024
b0ad378
added props to custom components
shubham1206agra Jan 17, 2024
756f220
Merge branch 'Expensify:main' into fix-form-submit
shubham1206agra Jan 22, 2024
93f21da
switch to form element instead of view
shubham1206agra Jan 22, 2024
4848e99
adjustment according to recommendations
shubham1206agra Jan 22, 2024
ab1eefa
Merge branch 'Expensify:main' into fix-form-submit
shubham1206agra Jan 23, 2024
55bb4d2
changes according to recommendation
shubham1206agra Jan 23, 2024
76e7862
merge main
shubham1206agra Jan 30, 2024
97c6da0
Merge branch 'Expensify:main' into fix-form-submit
shubham1206agra Jan 30, 2024
ae03b09
Compute the component-specific input registration params
cubuspl42 Jan 30, 2024
23ee294
minor fixes to logic
shubham1206agra Jan 31, 2024
18fa42a
adding custom component to the list
shubham1206agra Jan 31, 2024
f6f5339
fix lint
shubham1206agra Jan 31, 2024
3f8f417
minor fix to extra property passed
shubham1206agra Feb 1, 2024
daf4691
adjusted according to recommendations
shubham1206agra Feb 1, 2024
b12a526
minor adjustment
shubham1206agra Feb 1, 2024
78c3c27
Merge branch 'Expensify:main' into fix-form-submit
shubham1206agra Feb 3, 2024
cad60cf
merge main
shubham1206agra Feb 8, 2024
3124cb9
removed the support of AddressSearch from keyboardsubmit
shubham1206agra Feb 8, 2024
b500950
removed the old prop
shubham1206agra Feb 8, 2024
2951887
adjusted type
shubham1206agra Feb 8, 2024
326cd61
making textInputBasedComponents global to file
shubham1206agra Feb 8, 2024
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
13 changes: 11 additions & 2 deletions src/components/Form/FormProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import lodashIsEqual from 'lodash/isEqual';
import type {ForwardedRef, MutableRefObject, ReactNode} from 'react';
import React, {createRef, forwardRef, useCallback, useImperativeHandle, useMemo, useRef, useState} from 'react';
import type {NativeSyntheticEvent, TextInputSubmitEditingEventData} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import * as ValidationUtils from '@libs/ValidationUtils';
Expand Down Expand Up @@ -204,7 +205,7 @@ function FormProvider(
}));

const registerInput = useCallback<RegisterInput>(
<TInputProps extends BaseInputProps>(inputID: keyof Form, inputProps: TInputProps): TInputProps => {
<TInputProps extends BaseInputProps>(inputID: keyof Form, shouldSubmitForm: boolean, inputProps: TInputProps): TInputProps => {
const newRef: MutableRefObject<BaseInputProps> = inputRefs.current[inputID] ?? inputProps.ref ?? createRef();
if (inputRefs.current[inputID] !== newRef) {
inputRefs.current[inputID] = newRef;
Expand Down Expand Up @@ -232,6 +233,14 @@ function FormProvider(

return {
...inputProps,
...(shouldSubmitForm && {
onSubmitEditing: (event: NativeSyntheticEvent<TextInputSubmitEditingEventData>) => {
submit();

inputProps.onSubmitEditing?.(event);
},
returnKeyType: 'go',
}),
ref:
typeof inputRef === 'function'
? (node: BaseInputProps) => {
Expand Down Expand Up @@ -319,7 +328,7 @@ function FormProvider(
},
};
},
[draftValues, formID, errors, formState, hasServerError, inputValues, onValidate, setTouchedInput, shouldValidateOnBlur, shouldValidateOnChange],
[draftValues, inputValues, formState?.errorFields, errors, submit, setTouchedInput, shouldValidateOnBlur, onValidate, hasServerError, formID, shouldValidateOnChange],
);
const value = useMemo(() => ({registerInput}), [registerInput]);

Expand Down
7 changes: 3 additions & 4 deletions src/components/Form/FormWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {Keyboard, ScrollView} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import FormAlertWithSubmitButton from '@components/FormAlertWithSubmitButton';
import FormSubmit from '@components/FormSubmit';
import FormElement from '@components/FormElement';
import SafeAreaConsumer from '@components/SafeAreaConsumer';
import type {SafeAreaChildrenProps} from '@components/SafeAreaConsumer/types';
import ScrollViewWithContext from '@components/ScrollViewWithContext';
Expand Down Expand Up @@ -94,11 +94,10 @@ function FormWrapper({

const scrollViewContent = useCallback(
(safeAreaPaddingBottomStyle: SafeAreaChildrenProps['safeAreaPaddingBottomStyle']) => (
<FormSubmit
<FormElement
key={formID}
ref={formContentRef}
style={[style, safeAreaPaddingBottomStyle]}
onSubmit={onSubmit}
>
{children}
{isSubmitButtonVisible && (
Expand All @@ -116,7 +115,7 @@ function FormWrapper({
disablePressOnEnter={disablePressOnEnter}
/>
)}
</FormSubmit>
</FormElement>
),
[
children,
Expand Down
59 changes: 52 additions & 7 deletions src/components/Form/InputWrapper.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,66 @@
import type {ForwardedRef} from 'react';
import React, {forwardRef, useContext} from 'react';
import type {AnimatedTextInputRef} from '@components/RNTextInput';
import RoomNameInput from '@components/RoomNameInput';
import TextInput from '@components/TextInput';
import {canUseTouchScreen} from '@libs/DeviceCapabilities';
import FormContext from './FormContext';
import type {InputWrapperProps, ValidInputs} from './types';

function InputWrapper<TInput extends ValidInputs>({InputComponent, inputID, valueType = 'string', ...rest}: InputWrapperProps<TInput>, ref: ForwardedRef<AnimatedTextInputRef>) {
const textInputBasedComponents: ValidInputs[] = [TextInput, RoomNameInput];

function computeComponentSpecificRegistrationParams<TInput extends ValidInputs>({
InputComponent,
shouldSubmitForm,
multiline,
autoGrowHeight,
blurOnSubmit,
}: InputWrapperProps<TInput>): {
readonly shouldSubmitForm: boolean;
readonly blurOnSubmit: boolean | undefined;
readonly shouldSetTouchedOnBlurOnly: boolean;
} {
if (textInputBasedComponents.includes(InputComponent)) {
const isEffectivelyMultiline = Boolean(multiline) || Boolean(autoGrowHeight);

// If the user can use the hardware keyboard, they have access to an alternative way of inserting a new line
// (like a Shift+Enter keyboard shortcut). For simplicity, we assume that when there's no touch screen, it's a
// desktop setup with a keyboard.
const canUseHardwareKeyboard = !canUseTouchScreen();

// We want to avoid a situation when the user can't insert a new line. For single-line inputs, it's not a problem and we
// force-enable form submission. For multi-line inputs, ensure that it was requested to enable form submission for this specific
// input and that alternative ways exist to add a new line.
const shouldReallySubmitForm = isEffectivelyMultiline ? Boolean(shouldSubmitForm) && canUseHardwareKeyboard : true;

return {
// There are inputs that don't have onBlur methods, to simulate the behavior of onBlur in e.g. checkbox, we had to
// use different methods like onPress. This introduced a problem that inputs that have the onBlur method were
// calling some methods too early or twice, so we had to add this check to prevent that side effect.
// For now this side effect happened only in `TextInput` components.
shouldSetTouchedOnBlurOnly: true,
blurOnSubmit: (isEffectivelyMultiline && shouldReallySubmitForm) || blurOnSubmit,
shouldSubmitForm: shouldReallySubmitForm,
};
}

return {
shouldSetTouchedOnBlurOnly: false,
// Forward the originally provided value
blurOnSubmit,
shouldSubmitForm: false,
};
}

function InputWrapper<TInput extends ValidInputs>(props: InputWrapperProps<TInput>, ref: ForwardedRef<AnimatedTextInputRef>) {
const {InputComponent, inputID, valueType = 'string', shouldSubmitForm: propShouldSubmitForm, ...rest} = props;
const {registerInput} = useContext(FormContext);
// There are inputs that don't have onBlur methods, to simulate the behavior of onBlur in e.g. checkbox, we had to
// use different methods like onPress. This introduced a problem that inputs that have the onBlur method were
// calling some methods too early or twice, so we had to add this check to prevent that side effect.
// For now this side effect happened only in `TextInput` components.
const shouldSetTouchedOnBlurOnly = InputComponent === TextInput;

const {shouldSetTouchedOnBlurOnly, blurOnSubmit, shouldSubmitForm} = computeComponentSpecificRegistrationParams(props);

// TODO: Sometimes we return too many props with register input, so we need to consider if it's better to make the returned type more general and disregard the issue, or we would like to omit the unused props somehow.
// eslint-disable-next-line react/jsx-props-no-spreading, @typescript-eslint/no-explicit-any
return <InputComponent {...(registerInput(inputID, {ref, valueType, ...rest, shouldSetTouchedOnBlurOnly}) as any)} />;
return <InputComponent {...(registerInput(inputID, shouldSubmitForm, {ref, valueType, ...rest, shouldSetTouchedOnBlurOnly, blurOnSubmit}) as any)} />;
}

InputWrapper.displayName = 'InputWrapper';
Expand Down
14 changes: 12 additions & 2 deletions src/components/Form/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {ComponentProps, FocusEvent, Key, MutableRefObject, ReactNode, Ref} from 'react';
import type {GestureResponderEvent, NativeSyntheticEvent, StyleProp, TextInputFocusEventData, ViewStyle} from 'react-native';
import type {GestureResponderEvent, NativeSyntheticEvent, StyleProp, TextInputFocusEventData, TextInputSubmitEditingEventData, ViewStyle} from 'react-native';
import type AddressSearch from '@components/AddressSearch';
import type AmountTextInput from '@components/AmountTextInput';
import type CheckboxWithLabel from '@components/CheckboxWithLabel';
Expand Down Expand Up @@ -40,12 +40,22 @@ type BaseInputProps = {
isFocused?: boolean;
measureLayout?: (ref: unknown, callback: MeasureLayoutOnSuccessCallback) => void;
focus?: () => void;
multiline?: boolean;
autoGrowHeight?: boolean;
blurOnSubmit?: boolean;
onSubmitEditing?: (event: NativeSyntheticEvent<TextInputSubmitEditingEventData>) => void;
};

type InputWrapperProps<TInput extends ValidInputs> = Omit<BaseInputProps, 'ref'> &
ComponentProps<TInput> & {
InputComponent: TInput;
inputID: string;

/**
* Should the containing form be submitted when this input is submitted itself?
* Currently, meaningful only for text inputs.
*/
shouldSubmitForm?: boolean;
};

type ExcludeDraft<T> = T extends `${string}Draft` ? never : T;
Expand Down Expand Up @@ -89,7 +99,7 @@ type FormProps<TFormID extends OnyxFormKey = OnyxFormKey> = {
disablePressOnEnter?: boolean;
};

type RegisterInput = <TInputProps extends BaseInputProps>(inputID: keyof Form, inputProps: TInputProps) => TInputProps;
type RegisterInput = <TInputProps extends BaseInputProps>(inputID: keyof Form, shouldSubmitForm: boolean, inputProps: TInputProps) => TInputProps;

type InputRefs = Record<string, MutableRefObject<BaseInputProps>>;

Expand Down
18 changes: 0 additions & 18 deletions src/components/FormSubmit/index.native.tsx

This file was deleted.

86 changes: 0 additions & 86 deletions src/components/FormSubmit/index.tsx

This file was deleted.

13 changes: 0 additions & 13 deletions src/components/FormSubmit/types.ts

This file was deleted.

4 changes: 3 additions & 1 deletion src/components/RoomNameInput/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as RoomNameInputUtils from '@libs/RoomNameInputUtils';
import CONST from '@src/CONST';
import * as roomNameInputPropTypes from './roomNameInputPropTypes';

function RoomNameInput({isFocused, autoFocus, disabled, errorText, forwardedRef, value, onBlur, onChangeText, onInputChange, shouldDelayFocus}) {
function RoomNameInput({isFocused, autoFocus, disabled, errorText, forwardedRef, value, onBlur, onChangeText, onInputChange, onSubmitEditing, returnKeyType, shouldDelayFocus}) {
const {translate} = useLocalize();

const [selection, setSelection] = useState();
Expand Down Expand Up @@ -52,6 +52,8 @@ function RoomNameInput({isFocused, autoFocus, disabled, errorText, forwardedRef,
value={value.substring(1)} // Since the room name always starts with a prefix, we omit the first character to avoid displaying it twice.
selection={selection}
onSelectionChange={(event) => setSelection(event.nativeEvent.selection)}
onSubmitEditing={onSubmitEditing}
returnKeyType={returnKeyType}
errorText={errorText}
autoCapitalize="none"
onBlur={(event) => isFocused && onBlur(event)}
Expand Down
4 changes: 3 additions & 1 deletion src/components/RoomNameInput/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as RoomNameInputUtils from '@libs/RoomNameInputUtils';
import CONST from '@src/CONST';
import * as roomNameInputPropTypes from './roomNameInputPropTypes';

function RoomNameInput({isFocused, autoFocus, disabled, errorText, forwardedRef, value, onBlur, onChangeText, onInputChange, shouldDelayFocus}) {
function RoomNameInput({isFocused, autoFocus, disabled, errorText, forwardedRef, value, onBlur, onChangeText, onInputChange, onSubmitEditing, returnKeyType, shouldDelayFocus}) {
Copy link
Contributor

@cubuspl42 cubuspl42 Jan 22, 2024

Choose a reason for hiding this comment

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

I don't understand this change. We haven't adjusted the use of the RoomNameInput component, have we? Why is this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have changed the logic here as RoomNameInput has a TextInput to make it as close to the previous behavior.

const {translate} = useLocalize();

/**
Expand Down Expand Up @@ -42,6 +42,8 @@ function RoomNameInput({isFocused, autoFocus, disabled, errorText, forwardedRef,
maxLength={CONST.REPORT.MAX_ROOM_NAME_LENGTH}
keyboardType={keyboardType} // this is a bit hacky solution to a RN issue https://github.com/facebook/react-native/issues/27449
onBlur={(event) => isFocused && onBlur(event)}
onSubmitEditing={onSubmitEditing}
returnKeyType={returnKeyType}
autoFocus={isFocused && autoFocus}
autoCapitalize="none"
shouldDelayFocus={shouldDelayFocus}
Expand Down
8 changes: 8 additions & 0 deletions src/components/RoomNameInput/roomNameInputPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ const propTypes = {
/** A ref forwarded to the TextInput */
forwardedRef: refPropTypes,

/** On submit editing handler provided by the FormProvider */
onSubmitEditing: PropTypes.func,

/** Return key type provided to the TextInput */
returnKeyType: PropTypes.string,

/** The ID used to uniquely identify the input in a Form */
inputID: PropTypes.string,

Expand All @@ -39,6 +45,8 @@ const defaultProps = {
disabled: false,
errorText: '',
forwardedRef: () => {},
onSubmitEditing: () => {},
returnKeyType: undefined,

inputID: undefined,
onBlur: () => {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ const propTypes = {
/** Whether we should wait before focusing the TextInput, useful when using transitions */
shouldDelayFocus: PropTypes.bool,

/** Indicate whether pressing Enter on multiline input is allowed to submit the form. */
submitOnEnter: PropTypes.bool,

/** Indicate whether input is multiline */
multiline: PropTypes.bool,

Expand Down Expand Up @@ -132,7 +129,6 @@ const defaultProps = {
prefixCharacter: '',
onInputChange: () => {},
shouldDelayFocus: false,
submitOnEnter: false,
icon: null,
shouldUseDefaultValue: false,
multiline: false,
Expand Down
4 changes: 0 additions & 4 deletions src/components/TextInput/BaseTextInput/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ function BaseTextInput(
hint = '',
onInputChange = () => {},
shouldDelayFocus = false,
submitOnEnter = false,
multiline = false,
shouldInterceptSwipe = false,
autoCorrect = true,
Expand Down Expand Up @@ -376,9 +375,6 @@ function BaseTextInput(
selection={inputProps.selection}
readOnly={isReadOnly}
defaultValue={defaultValue}
// FormSubmit Enter key handler does not have access to direct props.
// `dataset.submitOnEnter` is used to indicate that pressing Enter on this input should call the submit callback.
dataSet={{submitOnEnter: isMultiline && submitOnEnter}}
/>
{inputProps.isLoading && (
<ActivityIndicator
Expand Down
Loading
Loading