-
Notifications
You must be signed in to change notification settings - Fork 3k
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 next button isn't aligned at bottom in switch to expensify reason form page #47957
Changes from all commits
6aeba43
68b569b
c00afa7
3ca16d1
a6cc298
1ed045a
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,5 +1,5 @@ | ||||||||||||||||||||||
import type {StackScreenProps} from '@react-navigation/stack'; | ||||||||||||||||||||||
import React, {useCallback, useEffect} from 'react'; | ||||||||||||||||||||||
import React, {useCallback, useEffect, useState} from 'react'; | ||||||||||||||||||||||
import {withOnyx} from 'react-native-onyx'; | ||||||||||||||||||||||
import FormProvider from '@components/Form/FormProvider'; | ||||||||||||||||||||||
import InputWrapper from '@components/Form/InputWrapper'; | ||||||||||||||||||||||
|
@@ -14,10 +14,13 @@ import useKeyboardState from '@hooks/useKeyboardState'; | |||||||||||||||||||||
import useLocalize from '@hooks/useLocalize'; | ||||||||||||||||||||||
import useNetwork from '@hooks/useNetwork'; | ||||||||||||||||||||||
import useSafeAreaInsets from '@hooks/useSafeAreaInsets'; | ||||||||||||||||||||||
import useSafePaddingBottomStyle from '@hooks/useSafePaddingBottomStyle'; | ||||||||||||||||||||||
import useStyledSafeAreaInsets from '@hooks/useStyledSafeAreaInsets'; | ||||||||||||||||||||||
import useStyleUtils from '@hooks/useStyleUtils'; | ||||||||||||||||||||||
import useThemeStyles from '@hooks/useThemeStyles'; | ||||||||||||||||||||||
import useWindowDimensions from '@hooks/useWindowDimensions'; | ||||||||||||||||||||||
import * as NumberUtils from '@libs/NumberUtils'; | ||||||||||||||||||||||
import StatusBar from '@libs/StatusBar'; | ||||||||||||||||||||||
import updateMultilineInputRange from '@libs/updateMultilineInputRange'; | ||||||||||||||||||||||
import Navigation from '@navigation/Navigation'; | ||||||||||||||||||||||
import type {SettingsNavigatorParamList} from '@navigation/types'; | ||||||||||||||||||||||
|
@@ -43,8 +46,19 @@ function ExitSurveyResponsePage({draftResponse, route, navigation}: ExitSurveyRe | |||||||||||||||||||||
const StyleUtils = useStyleUtils(); | ||||||||||||||||||||||
const {keyboardHeight} = useKeyboardState(); | ||||||||||||||||||||||
const {windowHeight} = useWindowDimensions(); | ||||||||||||||||||||||
const {top: safeAreaInsetsTop} = useSafeAreaInsets(); | ||||||||||||||||||||||
const {inputCallbackRef, inputRef} = useAutoFocusInput(); | ||||||||||||||||||||||
const [headerTitleHeight, setHeaderTitleHeight] = useState(0); | ||||||||||||||||||||||
|
||||||||||||||||||||||
// Device safe area top and bottom insets. | ||||||||||||||||||||||
// When the keyboard is shown, the bottom inset doesn't affect the height, so we take it out from the calculation. | ||||||||||||||||||||||
const {top: safeAreaInsetsTop, bottom: safeAreaInsetsBottom} = useSafeAreaInsets(); | ||||||||||||||||||||||
const safeAreaInsetsBottomValue = !keyboardHeight ? safeAreaInsetsBottom : 0; | ||||||||||||||||||||||
// FormWrapper bottom padding | ||||||||||||||||||||||
const {paddingBottom: formPaddingBottom} = useStyledSafeAreaInsets(); | ||||||||||||||||||||||
const formPaddingBottomValue = formPaddingBottom || styles.pb5.paddingBottom; | ||||||||||||||||||||||
// Extra bottom padding in FormAlertWithSubmitButton | ||||||||||||||||||||||
const safePaddingBottomStyle = useSafePaddingBottomStyle(); | ||||||||||||||||||||||
const safePaddingBottomStyleValue = 'paddingBottom' in safePaddingBottomStyle ? (safePaddingBottomStyle.paddingBottom as number) : 0; | ||||||||||||||||||||||
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. For the recent fix, I add another inset from The form currently has 3 bottom spacings. 1 from safe area insets (ScreenWrapper), 1 from styled safe area insets, App/src/components/Form/FormWrapper.tsx Lines 107 to 110 in 117b961
and the last 1 is from App/src/components/FormAlertWithSubmitButton.tsx Lines 99 to 100 in 117b961
The other 2 just add additional spacing in 2 different places and 2 different values. FormAlertWithSubmitButton which uses Lines 329 to 332 in 117b961
|
||||||||||||||||||||||
|
||||||||||||||||||||||
const {reason, backTo} = route.params; | ||||||||||||||||||||||
const {isOffline} = useNetwork({ | ||||||||||||||||||||||
|
@@ -71,7 +85,10 @@ function ExitSurveyResponsePage({draftResponse, route, navigation}: ExitSurveyRe | |||||||||||||||||||||
const textStyle = styles.headerAnonymousFooter; | ||||||||||||||||||||||
const baseResponseInputContainerStyle = styles.mt7; | ||||||||||||||||||||||
const formMaxHeight = Math.floor( | ||||||||||||||||||||||
windowHeight - | ||||||||||||||||||||||
// windowHeight doesn't include status bar height in Android, so we need to add it here. | ||||||||||||||||||||||
// StatusBar.currentHeight is only available on Android. | ||||||||||||||||||||||
windowHeight + | ||||||||||||||||||||||
(StatusBar.currentHeight ?? 0) - | ||||||||||||||||||||||
keyboardHeight - | ||||||||||||||||||||||
safeAreaInsetsTop - | ||||||||||||||||||||||
// Minus the height of HeaderWithBackButton | ||||||||||||||||||||||
|
@@ -81,16 +98,21 @@ function ExitSurveyResponsePage({draftResponse, route, navigation}: ExitSurveyRe | |||||||||||||||||||||
); | ||||||||||||||||||||||
const responseInputMaxHeight = NumberUtils.roundDownToLargestMultiple( | ||||||||||||||||||||||
formMaxHeight - | ||||||||||||||||||||||
safeAreaInsetsBottomValue - | ||||||||||||||||||||||
safePaddingBottomStyleValue - | ||||||||||||||||||||||
formPaddingBottomValue - | ||||||||||||||||||||||
// Minus the height of the text component | ||||||||||||||||||||||
textStyle.lineHeight - | ||||||||||||||||||||||
headerTitleHeight - | ||||||||||||||||||||||
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. One of the reasons the form errors overlaps is because the height of the title here is only a one-line height, but the title could be more than 1 line. The extra margin value (baseResponseInputContainerStyle.marginTop * 2) calculation below and the vertical margins (40) should be enough to cover the form error, but the form error ( |
||||||||||||||||||||||
// Minus the response input margins (multiplied by 2 to create the effect of margins on top and bottom). | ||||||||||||||||||||||
// marginBottom does not work in this case because the TextInput is in a ScrollView and will push the button beneath it out of view, | ||||||||||||||||||||||
// so it's maxHeight is what dictates space between it and the button. | ||||||||||||||||||||||
baseResponseInputContainerStyle.marginTop * 2 - | ||||||||||||||||||||||
// Minus the approximate size of a default button | ||||||||||||||||||||||
variables.componentSizeLarge - | ||||||||||||||||||||||
// Minus the vertical margins around the form button | ||||||||||||||||||||||
40, | ||||||||||||||||||||||
// Minus the height above the button for the form error text, accounting for 2 lines max. | ||||||||||||||||||||||
variables.lineHeightNormal * 2 - | ||||||||||||||||||||||
// Minus the margin between the button and the form error text | ||||||||||||||||||||||
styles.mb3.marginBottom, | ||||||||||||||||||||||
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. Instead of using 40 to create the effect of both the margin top and bottom, I changed it to use the error text line height multiplied by 2 to account for 2 lines of error and also a margin-bottom that exists between the error text and the button. The value now becomes (16-21 (scale depends on font size) * 2) + 12 = ~44-54 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. |
||||||||||||||||||||||
|
||||||||||||||||||||||
// Round down to the largest number of full lines | ||||||||||||||||||||||
styles.baseTextInput.lineHeight, | ||||||||||||||||||||||
|
@@ -120,7 +142,12 @@ function ExitSurveyResponsePage({draftResponse, route, navigation}: ExitSurveyRe | |||||||||||||||||||||
{isOffline && <ExitSurveyOffline />} | ||||||||||||||||||||||
{!isOffline && ( | ||||||||||||||||||||||
<> | ||||||||||||||||||||||
<Text style={textStyle}>{translate(`exitSurvey.prompts.${reason}`)}</Text> | ||||||||||||||||||||||
<Text | ||||||||||||||||||||||
style={textStyle} | ||||||||||||||||||||||
onLayout={(e) => setHeaderTitleHeight(e.nativeEvent.layout.height)} | ||||||||||||||||||||||
> | ||||||||||||||||||||||
{translate(`exitSurvey.prompts.${reason}`)} | ||||||||||||||||||||||
</Text> | ||||||||||||||||||||||
<InputWrapper | ||||||||||||||||||||||
InputComponent={TextInput} | ||||||||||||||||||||||
inputID={INPUT_IDS.RESPONSE} | ||||||||||||||||||||||
|
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.
useSafeAreaInsets
doesn't affect the height when the keyboard is shown.