-
Notifications
You must be signed in to change notification settings - Fork 3.9k
fix: autoGrow prop on mobile is handled by native platform #59078
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 all commits
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 |
|---|---|---|
|
|
@@ -18,12 +18,14 @@ import type {BaseTextInputProps, BaseTextInputRef} from '@components/TextInput/B | |
| import * as styleConst from '@components/TextInput/styleConst'; | ||
| import TextInputClearButton from '@components/TextInput/TextInputClearButton'; | ||
| import TextInputLabel from '@components/TextInput/TextInputLabel'; | ||
| import TextInputMeasurement from '@components/TextInput/TextInputMeasurement'; | ||
| import useHtmlPaste from '@hooks/useHtmlPaste'; | ||
| import useLocalize from '@hooks/useLocalize'; | ||
| import useMarkdownStyle from '@hooks/useMarkdownStyle'; | ||
| import useStyleUtils from '@hooks/useStyleUtils'; | ||
| import useTheme from '@hooks/useTheme'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import getPlatform from '@libs/getPlatform'; | ||
| import isInputAutoFilled from '@libs/isInputAutoFilled'; | ||
| import variables from '@styles/variables'; | ||
| import CONST from '@src/CONST'; | ||
|
|
@@ -80,6 +82,10 @@ function BaseTextInput( | |
| }: BaseTextInputProps, | ||
| ref: ForwardedRef<BaseTextInputRef>, | ||
| ) { | ||
| // For iOS, we don't need to measure the text input because it already has auto grow behavior | ||
| // See TextInputMeasurement.ios.tsx for more details | ||
| const isExternalAutoGrowMeasurement = getPlatform() !== CONST.PLATFORM.IOS && autoGrow; | ||
|
|
||
| const InputComponent = InputComponentMap.get(type) ?? RNTextInput; | ||
| const isMarkdownEnabled = type === 'markdown'; | ||
| const isAutoGrowHeightMarkdown = isMarkdownEnabled && autoGrowHeight; | ||
|
|
@@ -261,7 +267,7 @@ function BaseTextInput( | |
| styles.textInputContainer, | ||
| textInputContainerStyles, | ||
| !!contentWidth && StyleUtils.getWidthStyle(textInputWidth), | ||
| autoGrow && StyleUtils.getAutoGrowWidthInputContainerStyles(textInputWidth, autoGrowExtraSpace), | ||
| isExternalAutoGrowMeasurement && StyleUtils.getAutoGrowWidthInputContainerStyles(textInputWidth, autoGrowExtraSpace), | ||
| !hideFocusedState && isFocused && styles.borderColorFocus, | ||
| (!!hasError || !!errorText) && styles.borderColorDanger, | ||
| autoGrowHeight && {scrollPaddingTop: typeof maxAutoGrowHeight === 'number' ? 2 * maxAutoGrowHeight : undefined}, | ||
|
|
@@ -273,6 +279,10 @@ function BaseTextInput( | |
|
|
||
| // Height fix is needed only for Text single line inputs | ||
| const shouldApplyHeight = !isMultiline && !isMarkdownEnabled; | ||
|
|
||
| // Fix iOS cursor jumping when entering first character using HW keyboard https://github.com/Expensify/App/pull/59078#issuecomment-2802834037 | ||
| const selection = inputProps.selection?.end === 0 && inputProps.selection?.start === 0 ? undefined : inputProps.selection; | ||
|
|
||
| return ( | ||
| <> | ||
| <View style={[containerStyles]}> | ||
|
|
@@ -360,8 +370,8 @@ function BaseTextInput( | |
| placeholderTextColor={placeholderTextColor ?? theme.placeholderText} | ||
| underlineColorAndroid="transparent" | ||
| style={[ | ||
| styles.flex1, | ||
| styles.w100, | ||
| !autoGrow && styles.flex1, | ||
|
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. Can you explain why do we need to that this condition?
Contributor
Author
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. Yea sure, so previous solution (still present for Android) measured size of Now on iOS we must not have any width specified or native auto grow won't work. So we don't want to apply anything that will set width manually like
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. Thank you @Nodonisko |
||
| !autoGrow && styles.w100, | ||
| inputStyle, | ||
| (!hasLabel || isMultiline) && styles.pv0, | ||
| inputPaddingLeft, | ||
|
|
@@ -392,7 +402,7 @@ function BaseTextInput( | |
| keyboardType={inputProps.keyboardType} | ||
| inputMode={!disableKeyboard ? inputProps.inputMode : CONST.INPUT_MODE.NONE} | ||
| value={uncontrolled ? undefined : value} | ||
| selection={inputProps.selection} | ||
| selection={selection} | ||
| readOnly={isReadOnly} | ||
| defaultValue={defaultValue} | ||
| markdownStyle={markdownStyle} | ||
|
|
@@ -456,55 +466,21 @@ function BaseTextInput( | |
| /> | ||
| )} | ||
| </View> | ||
| {!!contentWidth && isPrefixCharacterPaddingCalculated && ( | ||
| <View | ||
| style={[inputStyle as ViewStyle, styles.hiddenElementOutsideOfWindow, styles.visibilityHidden, styles.wAuto, inputPaddingLeft]} | ||
| onLayout={(e) => { | ||
| if (e.nativeEvent.layout.width === 0 && e.nativeEvent.layout.height === 0) { | ||
| return; | ||
| } | ||
| setTextInputWidth(e.nativeEvent.layout.width); | ||
| setTextInputHeight(e.nativeEvent.layout.height); | ||
| }} | ||
| > | ||
| <Text | ||
| style={[ | ||
| inputStyle, | ||
| autoGrowHeight && styles.autoGrowHeightHiddenInput(width ?? 0, typeof maxAutoGrowHeight === 'number' ? maxAutoGrowHeight : undefined), | ||
| {width: contentWidth}, | ||
| ]} | ||
| > | ||
| {/* \u200B added to solve the issue of not expanding the text input enough when the value ends with '\n' (https://github.com/Expensify/App/issues/21271) */} | ||
| {value ? `${value}${value.endsWith('\n') ? '\u200B' : ''}` : placeholder} | ||
| </Text> | ||
| </View> | ||
| )} | ||
| {/* | ||
| Text input component doesn't support auto grow by default. | ||
| This text view is used to calculate width or height of the input value given textStyle in this component. | ||
| This Text component is intentionally positioned out of the screen. | ||
| */} | ||
| {(!!autoGrow || autoGrowHeight) && !isAutoGrowHeightMarkdown && ( | ||
| <Text | ||
| style={[ | ||
| inputStyle, | ||
| autoGrowHeight && styles.autoGrowHeightHiddenInput(width ?? 0, typeof maxAutoGrowHeight === 'number' ? maxAutoGrowHeight : undefined), | ||
| styles.hiddenElementOutsideOfWindow, | ||
| styles.visibilityHidden, | ||
| ]} | ||
| onLayout={(e) => { | ||
| if (e.nativeEvent.layout.width === 0 && e.nativeEvent.layout.height === 0) { | ||
| return; | ||
| } | ||
| // Add +2 to width so that cursor is not cut off / covered at the end of text content | ||
| setTextInputWidth(e.nativeEvent.layout.width + 2); | ||
| setTextInputHeight(e.nativeEvent.layout.height); | ||
| }} | ||
| > | ||
| {/* \u200B added to solve the issue of not expanding the text input enough when the value ends with '\n' (https://github.com/Expensify/App/issues/21271) */} | ||
| {value ? `${value}${value.endsWith('\n') ? '\u200B' : ''}` : placeholder} | ||
| </Text> | ||
| )} | ||
| <TextInputMeasurement | ||
| value={value} | ||
| placeholder={placeholder} | ||
| contentWidth={contentWidth} | ||
| autoGrowHeight={autoGrowHeight} | ||
| maxAutoGrowHeight={maxAutoGrowHeight} | ||
| width={width} | ||
| inputStyle={inputStyle} | ||
| inputPaddingLeft={inputPaddingLeft} | ||
| autoGrow={autoGrow} | ||
| isAutoGrowHeightMarkdown={isAutoGrowHeightMarkdown} | ||
| onSetTextInputWidth={setTextInputWidth} | ||
| onSetTextInputHeight={setTextInputHeight} | ||
| isPrefixCharacterPaddingCalculated={isPrefixCharacterPaddingCalculated} | ||
| /> | ||
| </> | ||
| ); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| function TextInputMeasurement() { | ||
| return null; | ||
| } | ||
|
|
||
| TextInputMeasurement.displayName = 'TextInputMeasurement'; | ||
|
|
||
| export default TextInputMeasurement; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| import React from 'react'; | ||
| import type {ViewStyle} from 'react-native'; | ||
| import {View} from 'react-native'; | ||
| import Text from '@components/Text'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import {isMobileChrome, isMobileSafari, isSafari} from '@libs/Browser'; | ||
| import type TextInputMeasurementProps from './types'; | ||
|
|
||
| function TextInputMeasurement({ | ||
| value, | ||
| placeholder, | ||
| contentWidth, | ||
| autoGrowHeight, | ||
| maxAutoGrowHeight, | ||
| width, | ||
| inputStyle, | ||
| inputPaddingLeft, | ||
| autoGrow, | ||
| isAutoGrowHeightMarkdown, | ||
| onSetTextInputWidth, | ||
| onSetTextInputHeight, | ||
| isPrefixCharacterPaddingCalculated, | ||
| }: TextInputMeasurementProps) { | ||
| const styles = useThemeStyles(); | ||
|
|
||
| return ( | ||
| <> | ||
| {!!contentWidth && isPrefixCharacterPaddingCalculated && ( | ||
| <View | ||
| style={[inputStyle as ViewStyle, styles.hiddenElementOutsideOfWindow, styles.visibilityHidden, styles.wAuto, inputPaddingLeft]} | ||
| onLayout={(e) => { | ||
| if (e.nativeEvent.layout.width === 0 && e.nativeEvent.layout.height === 0) { | ||
| return; | ||
| } | ||
| onSetTextInputWidth(e.nativeEvent.layout.width); | ||
| onSetTextInputHeight(e.nativeEvent.layout.height); | ||
| }} | ||
| > | ||
| <Text | ||
| style={[ | ||
| inputStyle, | ||
| autoGrowHeight && styles.autoGrowHeightHiddenInput(width ?? 0, typeof maxAutoGrowHeight === 'number' ? maxAutoGrowHeight : undefined), | ||
| {width: contentWidth}, | ||
| ]} | ||
| > | ||
| {/* \u200B added to solve the issue of not expanding the text input enough when the value ends with '\n' (https://github.com/Expensify/App/issues/21271) */} | ||
| {value ? `${value}${value.endsWith('\n') ? '\u200B' : ''}` : placeholder} | ||
| </Text> | ||
| </View> | ||
| )} | ||
| {/* | ||
| Text input component doesn't support auto grow by default. | ||
| We're using a hidden text input to achieve that. | ||
| This text view is used to calculate width or height of the input value given textStyle in this component. | ||
| This Text component is intentionally positioned out of the screen. | ||
| */} | ||
| {(!!autoGrow || !!autoGrowHeight) && !isAutoGrowHeightMarkdown && ( | ||
| // Add +2 to width on Safari browsers so that text is not cut off due to the cursor or when changing the value | ||
| // Reference: https://github.com/Expensify/App/issues/8158, https://github.com/Expensify/App/issues/26628 | ||
| // For mobile Chrome, ensure proper display of the text selection handle (blue bubble down). | ||
| // Reference: https://github.com/Expensify/App/issues/34921 | ||
| <Text | ||
| style={[ | ||
|
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. From #76785, the displayed input and hidden measurement text don’t match due to styling differences, so their styles need to be synced |
||
| inputStyle, | ||
| autoGrowHeight && styles.autoGrowHeightHiddenInput(width ?? 0, typeof maxAutoGrowHeight === 'number' ? maxAutoGrowHeight : undefined), | ||
| styles.hiddenElementOutsideOfWindow, | ||
| styles.visibilityHidden, | ||
| ]} | ||
| onLayout={(e) => { | ||
| if (e.nativeEvent.layout.width === 0 && e.nativeEvent.layout.height === 0) { | ||
| return; | ||
| } | ||
| let additionalWidth = 0; | ||
| if (isMobileSafari() || isSafari() || isMobileChrome()) { | ||
| additionalWidth = 2; | ||
| } | ||
|
Comment on lines
73
to
76
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. Previously, we also set additionalWidth = 2 on Android native. Can you confirm if we don't need it for Android anymore?
Contributor
Author
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. |
||
| onSetTextInputWidth(e.nativeEvent.layout.width + additionalWidth); | ||
| onSetTextInputHeight(e.nativeEvent.layout.height); | ||
| }} | ||
| > | ||
| {/* \u200B added to solve the issue of not expanding the text input enough when the value ends with '\n' (https://github.com/Expensify/App/issues/21271) */} | ||
| {value ? `${value}${value.endsWith('\n') ? '\u200B' : ''}` : placeholder} | ||
| </Text> | ||
| )} | ||
| </> | ||
| ); | ||
| } | ||
|
|
||
| TextInputMeasurement.displayName = 'TextInputMeasurement'; | ||
|
|
||
| export default TextInputMeasurement; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import type {StyleProp, TextStyle, ViewStyle} from 'react-native'; | ||
|
|
||
| type TextInputMeasurementProps = { | ||
| /** The value to measure */ | ||
| value?: string; | ||
|
|
||
| /** The placeholder to measure */ | ||
| placeholder?: string; | ||
|
|
||
| /** The width to measure */ | ||
| contentWidth?: number; | ||
|
|
||
| /** Whether to auto grow height */ | ||
| autoGrowHeight?: boolean; | ||
|
|
||
| /** The maximum height for auto grow */ | ||
| maxAutoGrowHeight?: number; | ||
|
|
||
| /** The width of the container */ | ||
| width: number | null; | ||
|
|
||
| /** The input style */ | ||
| inputStyle?: StyleProp<TextStyle>; | ||
|
|
||
| /** The input padding left */ | ||
| inputPaddingLeft?: StyleProp<ViewStyle>; | ||
|
|
||
| /** Whether to auto grow */ | ||
| autoGrow?: boolean; | ||
|
|
||
| /** Whether the input is markdown */ | ||
| isAutoGrowHeightMarkdown?: boolean; | ||
|
|
||
| /** Callback to set the text input width */ | ||
| onSetTextInputWidth: (width: number) => void; | ||
|
|
||
| /** Callback to set the text input height */ | ||
| onSetTextInputHeight: (height: number) => void; | ||
|
|
||
| /** Whether the prefix character padding is calculated */ | ||
| isPrefixCharacterPaddingCalculated: boolean; | ||
| }; | ||
|
|
||
| export default TextInputMeasurementProps; |

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change caused a bug. Details in #60640