Skip to content

Commit

Permalink
Merge pull request #14409 from margelo/fix/scroll-to-picker-regression
Browse files Browse the repository at this point in the history
Fix: Scroll to picker regression
  • Loading branch information
aldo-expensify authored Jan 24, 2023
2 parents 4ddb47e + 3392ff1 commit 6f70a7d
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 35 deletions.
12 changes: 12 additions & 0 deletions contributingGuides/FORMS.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,15 @@ Any `Form.js` that has a button will also add safe area padding by default. If t
</Form>
</ScreenWrapper>
```

### Handling nested Pickers in Form

In case there's a nested Picker in Form, we should pass the props below to Form, as needed:

#### Enable ScrollContext

Pass the `scrollContextEnabled` prop to enable scrolling up when Picker is pressed, making sure the Picker is always in view and doesn't get covered by virtual keyboards for example.

#### Enable scrolling to overflow

In addition to the `scrollContextEnabled` prop, we can also pass `scrollToOverflowEnabled` when the nested Picker is at the bottom of the Form to prevent the popup selector from covering Picker.
93 changes: 61 additions & 32 deletions src/components/Form.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import lodashGet from 'lodash/get';
import React from 'react';
import {View} from 'react-native';
import {View, ScrollView} from 'react-native';
import PropTypes from 'prop-types';
import _ from 'underscore';
import {withOnyx} from 'react-native-onyx';
Expand Down Expand Up @@ -56,6 +56,17 @@ const propTypes = {
/** Whether the form submit action is dangerous */
isSubmitActionDangerous: PropTypes.bool,

/** Whether the ScrollView overflow content is scrollable.
* Set to true to avoid nested Picker components at the bottom of the Form from rendering the popup selector over Picker
* e.g. https://github.com/Expensify/App/issues/13909#issuecomment-1396859008
*/
scrollToOverflowEnabled: PropTypes.bool,

/** Whether ScrollWithContext should be used instead of regular ScrollView.
* Set to true when there's a nested Picker component in Form.
*/
scrollContextEnabled: PropTypes.bool,

...withLocalizePropTypes,
};

Expand All @@ -68,6 +79,8 @@ const defaultProps = {
draftValues: {},
enabledWhenOffline: false,
isSubmitActionDangerous: false,
scrollToOverflowEnabled: false,
scrollContextEnabled: false,
};

class Form extends React.Component {
Expand All @@ -79,6 +92,7 @@ class Form extends React.Component {
inputValues: {},
};

this.formRef = React.createRef(null);
this.inputRefs = {};
this.touchedInputs = {};

Expand Down Expand Up @@ -258,45 +272,60 @@ class Form extends React.Component {
}

render() {
const scrollViewContent = safeAreaPaddingBottomStyle => (
<View style={[this.props.style, safeAreaPaddingBottomStyle]}>
{this.childrenWrapperWithProps(this.props.children)}
{this.props.isSubmitButtonVisible && (
<FormAlertWithSubmitButton
buttonText={this.props.submitButtonText}
isAlertVisible={_.size(this.state.errors) > 0 || Boolean(this.getErrorMessage()) || !_.isEmpty(this.props.formState.errorFields)}
isLoading={this.props.formState.isLoading}
message={_.isEmpty(this.props.formState.errorFields) ? this.getErrorMessage() : null}
onSubmit={this.submit}
onFixTheErrorsLinkPressed={() => {
const errors = !_.isEmpty(this.state.errors) ? this.state.errors : this.props.formState.errorFields;
const focusKey = _.find(_.keys(this.inputRefs), key => _.keys(errors).includes(key));
const focusInput = this.inputRefs[focusKey];
if (focusInput.focus && typeof focusInput.focus === 'function') {
focusInput.focus();
}

// We subtract 10 to scroll slightly above the input
if (focusInput.measureLayout && typeof focusInput.measureLayout === 'function') {
focusInput.measureLayout(this.formRef.current, (x, y) => this.formRef.current.scrollTo({y: y - 10, animated: false}));
}
}}
containerStyles={[styles.mh0, styles.mt5, styles.flex1]}
enabledWhenOffline={this.props.enabledWhenOffline}
isSubmitActionDangerous={this.props.isSubmitActionDangerous}
/>
)}
</View>
);

return (
<SafeAreaConsumer>
{({safeAreaPaddingBottomStyle}) => (
{({safeAreaPaddingBottomStyle}) => (this.props.scrollContextEnabled ? (
<ScrollViewWithContext
style={[styles.w100, styles.flex1]}
contentContainerStyle={styles.flexGrow1}
keyboardShouldPersistTaps="handled"
ref={el => this.form = el}
scrollToOverflowEnabled={this.props.scrollToOverflowEnabled}
ref={this.formRef}
>
<View style={[this.props.style, safeAreaPaddingBottomStyle]}>
{this.childrenWrapperWithProps(this.props.children)}
{this.props.isSubmitButtonVisible && (
<FormAlertWithSubmitButton
buttonText={this.props.submitButtonText}
isAlertVisible={_.size(this.state.errors) > 0 || Boolean(this.getErrorMessage()) || !_.isEmpty(this.props.formState.errorFields)}
isLoading={this.props.formState.isLoading}
message={_.isEmpty(this.props.formState.errorFields) ? this.getErrorMessage() : null}
onSubmit={this.submit}
onFixTheErrorsLinkPressed={() => {
const errors = !_.isEmpty(this.state.errors) ? this.state.errors : this.props.formState.errorFields;
const focusKey = _.find(_.keys(this.inputRefs), key => _.keys(errors).includes(key));
const focusInput = this.inputRefs[focusKey];
if (focusInput.focus && typeof focusInput.focus === 'function') {
focusInput.focus();
}

// We subtract 10 to scroll slightly above the input
if (focusInput.measureLayout && typeof focusInput.measureLayout === 'function') {
focusInput.measureLayout(this.form, (x, y) => this.form.scrollTo({y: y - 10, animated: false}));
}
}}
containerStyles={[styles.mh0, styles.mt5, styles.flex1]}
enabledWhenOffline={this.props.enabledWhenOffline}
isSubmitActionDangerous={this.props.isSubmitActionDangerous}
/>
)}
</View>
{scrollViewContent(safeAreaPaddingBottomStyle)}
</ScrollViewWithContext>
)}
) : (
<ScrollView
style={[styles.w100, styles.flex1]}
contentContainerStyle={styles.flexGrow1}
keyboardShouldPersistTaps="handled"
scrollToOverflowEnabled={this.props.scrollToOverflowEnabled}
ref={this.formRef}
>
{scrollViewContent(safeAreaPaddingBottomStyle)}
</ScrollView>
))}
</SafeAreaConsumer>
);
}
Expand Down
9 changes: 6 additions & 3 deletions src/components/ScrollViewWithContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class ScrollViewWithContext extends React.Component {
this.state = {
contentOffsetY: 0,
};
this.scrollViewRef = React.createRef(null);
this.scrollViewRef = this.props.innerRef || React.createRef(null);

this.setContextScrollPosition = this.setContextScrollPosition.bind(this);
}
Expand All @@ -42,7 +42,6 @@ class ScrollViewWithContext extends React.Component {
ref={this.scrollViewRef}
onScroll={this.setContextScrollPosition}
scrollEventThrottle={this.props.scrollEventThrottle || MIN_SMOOTH_SCROLL_EVENT_THROTTLE}
scrollToOverflowEnabled
>
<ScrollContext.Provider
value={{
Expand All @@ -58,7 +57,11 @@ class ScrollViewWithContext extends React.Component {
}
ScrollViewWithContext.propTypes = propTypes;

export default ScrollViewWithContext;
export default React.forwardRef((props, ref) => (
// eslint-disable-next-line react/jsx-props-no-spreading
<ScrollViewWithContext {...props} innerRef={ref} />
));

export {
ScrollContext,
};
1 change: 1 addition & 0 deletions src/pages/AddPersonalBankAccountPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ class AddPersonalBankAccountPage extends React.Component {
formID={ONYXKEYS.PERSONAL_BANK_ACCOUNT}
isSubmitButtonVisible={Boolean(this.state.selectedPlaidAccountID)}
submitButtonText={this.props.translate('common.saveAndContinue')}
scrollContextEnabled
onSubmit={this.submit}
validate={this.validate}
style={[styles.mh5, styles.flex1]}
Expand Down
2 changes: 2 additions & 0 deletions src/pages/EnablePayments/AdditionalDetailsStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ class AdditionalDetailsStep extends React.Component {
formID={ONYXKEYS.WALLET_ADDITIONAL_DETAILS}
validate={this.validate}
onSubmit={this.activateWallet}
scrollContextEnabled
scrollToOverflowEnabled
submitButtonText={this.props.translate('common.saveAndContinue')}
style={[styles.mh5, styles.flexGrow1]}
>
Expand Down
1 change: 1 addition & 0 deletions src/pages/ReimbursementAccount/BankAccountPlaidStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class BankAccountPlaidStep extends React.Component {
formID={ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM}
validate={() => ({})}
onSubmit={this.submit}
scrollContextEnabled
submitButtonText={this.props.translate('common.saveAndContinue')}
style={[styles.mh5, styles.flexGrow1]}
isSubmitButtonVisible={Boolean(selectedPlaidAccountID) && !_.isEmpty(lodashGet(this.props.plaidData, 'bankAccounts'))}
Expand Down
2 changes: 2 additions & 0 deletions src/pages/ReimbursementAccount/CompanyStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ class CompanyStep extends React.Component {
formID={ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM}
validate={this.validate}
onSubmit={this.submit}
scrollContextEnabled
scrollToOverflowEnabled
submitButtonText={this.props.translate('common.saveAndContinue')}
style={[styles.ph5, styles.flexGrow1]}
>
Expand Down
1 change: 1 addition & 0 deletions src/pages/ReimbursementAccount/RequestorStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ class RequestorStep extends React.Component {
formID={ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM}
submitButtonText={this.props.translate('common.saveAndContinue')}
validate={this.validate}
scrollContextEnabled
onSubmit={this.submit}
style={[styles.mh5, styles.flexGrow1]}
>
Expand Down
1 change: 1 addition & 0 deletions src/pages/ReportSettingsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class ReportSettingsPage extends Component {
style={[styles.mh5, styles.mt5, styles.flexGrow1]}
validate={this.validate}
onSubmit={this.updatePolicyRoomName}
scrollContextEnabled
isSubmitButtonVisible={shouldShowRoomName && !shouldDisableRename}
enabledWhenOffline
>
Expand Down
2 changes: 2 additions & 0 deletions src/pages/settings/Payments/AddDebitCardPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ class DebitCardPage extends Component {
validate={this.validate}
onSubmit={PaymentMethods.addPaymentCard}
submitButtonText={this.props.translate('common.save')}
scrollContextEnabled
scrollToOverflowEnabled
style={[styles.mh5, styles.flexGrow1]}
>
<TextInput
Expand Down
1 change: 1 addition & 0 deletions src/pages/workspace/WorkspaceNewRoomPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class WorkspaceNewRoomPage extends React.Component {
<Form
formID={ONYXKEYS.FORMS.NEW_ROOM_FORM}
submitButtonText={this.props.translate('newRoomPage.createRoom')}
scrollContextEnabled
style={[styles.mh5, styles.mt5, styles.flexGrow1]}
validate={this.validate}
onSubmit={this.submit}
Expand Down
1 change: 1 addition & 0 deletions src/pages/workspace/WorkspaceSettingsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class WorkspaceSettingsPage extends React.Component {
formID={ONYXKEYS.FORMS.WORKSPACE_SETTINGS_FORM}
submitButtonText={this.props.translate('workspace.editor.save')}
style={[styles.mh5, styles.flexGrow1]}
scrollContextEnabled
validate={this.validate}
onSubmit={this.submit}
enabledWhenOffline
Expand Down

0 comments on commit 6f70a7d

Please sign in to comment.