-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix: Scroll to picker regression #14409
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
Changes from 19 commits
c4e8ea8
71a415a
384eedb
1cbeaa7
fe8d4cd
65d5e9c
7ba22bd
8172dce
8edc865
3505fd9
b7ff191
8cba034
27f5fea
165af16
7d59c4b
b08dc2b
8c479d2
a0e9574
bf7f0da
c37e279
63ff50a
3392ff1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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'; | ||
|
|
@@ -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, | ||
luacmartins marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ...withLocalizePropTypes, | ||
| }; | ||
|
|
||
|
|
@@ -68,6 +79,8 @@ const defaultProps = { | |
| draftValues: {}, | ||
| enabledWhenOffline: false, | ||
| isSubmitActionDangerous: false, | ||
| scrollToOverflowEnabled: false, | ||
| scrollContextEnabled: false, | ||
| }; | ||
|
|
||
| class Form extends React.Component { | ||
|
|
@@ -79,6 +92,7 @@ class Form extends React.Component { | |
| inputValues: {}, | ||
| }; | ||
|
|
||
| this.formRef = React.createRef(null); | ||
| this.inputRefs = {}; | ||
| this.touchedInputs = {}; | ||
|
|
||
|
|
@@ -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.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> | ||
| ); | ||
|
|
||
| return ( | ||
| <SafeAreaConsumer> | ||
| {({safeAreaPaddingBottomStyle}) => ( | ||
| {({safeAreaPaddingBottomStyle}) => (this.props.scrollContextEnabled ? ( | ||
|
Contributor
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. @aldo-expensify, @chrispader Here could've been a better implementation instead of DRY code , something like const ScrollViewComponent = this.props.scrollContextEnabled ? ScrollViewWithContext : ScrollView;
Contributor
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. I agree, that would have resulted in nicer code 🥲 |
||
| <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> | ||
| ); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.