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

@rayane-djouah

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:

Open the app and log in
Navigate to settings > Switch to Expensify Classic
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?

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

@JmillsExpensify

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.

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

@JmillsExpensify

@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

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.

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) - ...

@JmillsExpensify

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

@JmillsExpensify

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

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

@rayane-djouah

PR is ready

cc: @rayane-djouah

@JmillsExpensify

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

@JmillsExpensify

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

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

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

Do we agree 👍 or 👎

@JmillsExpensify

@JmillsExpensify Friendly bump

@bernhardoj

Payment summary:

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

@rayane-djouah

Offer sent for @rayane-djouah

@JmillsExpensify

@JmillsExpensify Offer accepted, Thanks

@bernhardoj

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.

Requested in ND.

@bernhardoj

Payment summary:

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

@bernhardoj

$250 approved for @bernhardoj

Navigation Menu

Repository navigation

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

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

Proposal

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

What is the root cause of that problem?

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

12 remaining items

Checklist

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

	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,

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

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

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Aug 13, 2024

lanitochka17 commented on Aug 13, 2024

lanitochka17 commented on Aug 13, 2024

daledah commented on Aug 13, 2024

Proposal

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

What is the root cause of that problem?

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

bernhardoj commented on Aug 13, 2024

Proposal

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

What is the root cause of that problem?

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

melvin-bot commented on Aug 16, 2024

melvin-bot commented on Aug 20, 2024

melvin-bot commented on Aug 21, 2024

12 remaining items

bernhardoj commented on Aug 24, 2024

rayane-d commented on Sep 9, 2024

rayane-d commented on Sep 10, 2024

rayane-d commented on Sep 17, 2024

Checklist

rayane-d commented on Sep 17, 2024

JmillsExpensify commented on Sep 18, 2024

JmillsExpensify commented on Sep 18, 2024

rayane-d commented on Sep 18, 2024

JmillsExpensify commented on Sep 18, 2024

bernhardoj commented on Sep 18, 2024

garrettmknight commented on Sep 18, 2024

JmillsExpensify commented on Sep 18, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions