Android - Switch to Expesify classic - Next button has extra padding on 2nd screen of survey

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.19.3
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #47025

Action Performed:

1. Open the app and log in
2. Navigate to settings > Switch to Expensify Classic
3. Select any reason and proceed to the next screen

Expected Result:

The next button on all screens of the survey has the same padding

Actual Result:

The Next button has an extra padding on the 2nd screen of the survey ("Why do you prefer Expensify Classic?")

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6570282_1723492036339.video_2024-08-12_15-47-06.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @rayane-djouah

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

We think that this bug might be related to #vip-vsp

[lanitochka17]

@JmillsExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[daledah]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Android - Switch to Expesify classic - Next button has extra padding on 2nd screen of survey

What is the root cause of that problem?

The StyleUtils.getMaximumHeight(formMaxHeight) makes the form's height is shorter than normal, so the button is moved to top as well:

App/src/pages/settings/ExitSurvey/ExitSurveyResponsePage.tsx

Line 107 in 3b0f99e

style={[styles.flex1, styles.mh5, formTopMarginsStyle, StyleUtils.getMaximumHeight(formMaxHeight)]}

What changes do you think we should make in order to solve the problem?

We should remove that style.

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Next button of 2nd screen of survey on Android has extra spacing below it compares to other platform.

What is the root cause of that problem?

On the 2nd screen of the survey page, we have a max height style to limit the form height.

App/src/pages/settings/ExitSurvey/ExitSurveyResponsePage.tsx

Line 107 in 420c2a1

style={[styles.flex1, styles.mh5, formTopMarginsStyle, StyleUtils.getMaximumHeight(formMaxHeight)]}

The max height value in Android has a wrong calculation that leads to the form not taking all available space. The form height is used to let the form input to expand based on the form height.

App/src/pages/settings/ExitSurvey/ExitSurveyResponsePage.tsx

Lines 73 to 81 in 420c2a1

	const formMaxHeight = Math.floor(
	windowHeight -
	keyboardHeight -
	safeAreaInsetsTop -
	// Minus the height of HeaderWithBackButton
	variables.contentHeaderHeight -
	// Minus the top margins on the form
	formTopMarginsStyle.marginTop,
	);

The form height gets the window height and then subtracts it from the inset top. However, the window height on Android doesn't take the inset top height into account, so subtracting it from the inset top will make it smaller. We face this similar issue in #14563 too.

Another issue I noticed is that on iOS, the input can overlap with the button.

This is because recently, we have a PR that adds another bottom space for the button, but we didn't include the calculation yet to the max input height calculation.

App/src/pages/settings/ExitSurvey/ExitSurveyResponsePage.tsx

Lines 82 to 93 in 420c2a1

	const responseInputMaxHeight = NumberUtils.roundDownToLargestMultiple(
	formMaxHeight -
	// Minus the height of the text component
	textStyle.lineHeight -
	// 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,

What changes do you think we should make in order to solve the problem?

Add StatusBar.currentHeight to window height. StatusBar.currentHeight doesn't exist on other platforms.

const formMaxHeight = Math.floor(
        windowHeight + (StatusBar.currentHeight ?? 0) -

OR

We can remove the formMaxHeight and the max height style and get the form height from the form onLayout.

const [formHeight, setFormHeight] = useState(0);
const responseInputMaxHeight = NumberUtils.roundDownToLargestMultiple(
    formHeight - ...
);

<FormProvider
    onLayout={e => setFormHeight(e.nativeEvent.layout.height)}

The onLayout will be used in FormWrapper > FormElement.

App/src/components/Form/FormWrapper.tsx

Lines 107 to 111 in 420c2a1

	<FormElement
	key={formID}
	ref={formContentRef}
	style={[style, safeAreaPaddingBottomStyle.paddingBottom ? safeAreaPaddingBottomStyle : styles.pb5]}
	>

But the initial value of formHeight is 0, so there could be some "blinking".

---

To fix the iOS issue, we need to include the bottom padding to responseInputMaxHeight.

const {top: safeAreaInsetsTop, bottom: safeAreaInsetsBottom} = useSafeAreaInsets();
const safePaddingBottomStyle = useSafePaddingBottomStyle();

const responseInputMaxHeight = NumberUtils.roundDownToLargestMultiple(
        formMaxHeight - safeAreaInsetsBottom - (safePaddingBottomStyle.paddingBottom ?? 0) - ...

[melvin-bot]

@JmillsExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@JmillsExpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rayane-djouah (External)

[JmillsExpensify]

@rayane-djouah proposals for review above when you get a chance.

[rayane-djouah]

@bernhardoj - Thanks for the detailed proposal and for catching the iOS "input overlap with the button" bug. I agree with your proposed RCA and solution.

Note: For the "input overlap with the button" bug, we need to consider the input and button messages in responseInputMaxHeight calculation:

@bernhardoj's proposal looks good to me.
🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @roryabraham, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[bernhardoj]

PR is ready

cc: @rayane-djouah

[rayane-djouah]

Note

The production deploy automation failed: This should be on [HOLD for Payment 2024-09-12] according to #48515 production deploy checklist, confirmed in #47957 (comment)

cc @JmillsExpensify

[rayane-djouah]

@JmillsExpensify Friendly bump on updating the issue labels and title. Thanks!

[rayane-djouah]

Checklist

• The PR that introduced the bug has been identified. Link to the PR: Mandatory exit survey for users going back to OldDot #34925
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/34925/files#r1763950750
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• Determine if we should create a regression test for this bug. Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Open Settings > Switch to Expensify Classic.
2. Select any reason and proceed to the next screen.
3. Verify the next button is aligned at the bottom of the page.
4. The next button should have the same padding on all screens of the survey flow.

Do we agree 👍 or 👎

[rayane-djouah]

@JmillsExpensify Friendly bump

[JmillsExpensify]

Payment summary:

• Contributor: @bernhardoj $250
• Reviewer: @rayane-djouah $250

[JmillsExpensify]

Offer sent for @rayane-djouah

[rayane-djouah]

@JmillsExpensify Offer accepted, Thanks

[JmillsExpensify]

All paid out. I'm going to pass on a regression test for this one given the polish nature of this bug. @bernhardoj please submit a request via New Expensify and we'll make sure your payment is processed.

[bernhardoj]

Requested in ND.

[garrettmknight]

Payment summary:

• Contributor: @bernhardoj $250
• Reviewer: @rayane-djouah $250 paid via Upwork.

[JmillsExpensify]

$250 approved for @bernhardoj