[$125] [Ideal nav] Android - No bottom margin under the sign out button

[kavimuru]

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: 1.4.59-0
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
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

Action Performed:

1. Navigate to staging.new.expensify.com
2. Click on settings
3. Scroll down to the bottom of the page
4. Hold on the sign out button (don't click it, but hold on it)

Expected Result:

There is a bottom margin under the sign out button

Actual Result:

There is no bottom margin under the sign out button which is inconsistent with other platforms like mWeb

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

Bug6435986_1712086689970.Screen_Recording_20240402_223437_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0153b4e342ae26d6b3
• Upwork Job ID: 1775442319575195648
• Last Price Increase: 2024-04-12
• Automatic offers:
• Krishna2323 | Contributor | 0

[melvin-bot]

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[kavimuru]

@sonialiap 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.

[Krishna2323]

Proposal

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

Android - No bottom margin under the sign out button

What is the root cause of that problem?

We don't have padding bottom applied to ScrollView.

App/src/pages/settings/InitialSettingsPage.tsx

Lines 479 to 485 in 935df86

	<ScrollView
	ref={scrollViewRef}
	onLayout={onLayout}
	onScroll={onScroll}
	scrollEventThrottle={16}
	// We use marginTop to prevent glitching on the initial frame that renders before scrollTo.
	contentContainerStyle={[!isAfterOnLayout && !!scrollOffset && {marginTop: -scrollOffset}]}

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

We need to add padding/margin bottom to contentContainerStyle. We may want to apply pb4 but that can be discussed.

Optionally:
We can also apply flexGrow1 to ScrollView.
We can remove pt4 from style prop it to contentContainerStyle.

App/src/pages/settings/InitialSettingsPage.tsx

Line 486 in 935df86

style={[styles.w100, styles.pt4]}

Result

[ahmedGaber93]

This seems a regression from #38274 which added style styles.pt4 to scrollview that move the scrollview content bottom with this padding value, I think the style should set to contentContainerStyle instead of style.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0153b4e342ae26d6b3

[melvin-bot]

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

[Krishna2323]

Proposal updated

• Added reference

[nayabatir1]

Proposal

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

Android - No bottom margin under the sign out button

iOS also has this padding issue

What is the root cause of that problem?

Bottom padding is missing scroll view container

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

App/src/pages/settings/InitialSettingsPage.tsx

Lines 479 to 486 in 935df86

	<ScrollView
	ref={scrollViewRef}
	onLayout={onLayout}
	onScroll={onScroll}
	scrollEventThrottle={16}
	// We use marginTop to prevent glitching on the initial frame that renders before scrollTo.
	contentContainerStyle={[!isAfterOnLayout && !!scrollOffset && {marginTop: -scrollOffset}]}
	style={[styles.w100, styles.pt4]}

On adding bottom padding to style, it didn't fix the issue

style={[styles.w100, styles.pt4, styles.pb4]}

Instead added vertical padding to content container style it fixed for both ios and android, and removed padding from style.
Using padding vertical for consistent padding vertically instead of using top and bottom separately.

contentContainerStyle={[!isAfterOnLayout && !!scrollOffset && {marginTop: -scrollOffset}, styles.pv4]}
style={styles.w100}

THIS CODE IS TESTED LOCALLY

What alternative solutions did you explore? (Optional)

N.A.

[trjExpensify]

CC: @Expensify/design is this a problem?

[dannymcclain]

CC: @Expensify/design is this a problem?

I would say this is a very very small problem. Ideally yes there should be some margin below that nav item, but I'd consider it pretty low priority since it doesn't break anything visually or functionally.

[ahmedGaber93]

Proposal

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

Android - No bottom margin under the sign out button

What is the root cause of that problem?

This is a regression from #38274
we added here styles.pt4 to scrollview here, which move the scrollview content bottom with this padding value

App/src/pages/settings/InitialSettingsPage.tsx

Line 486 in 935df86

style={[styles.w100, styles.pt4]}

Section view already have styles.pb4 here

App/src/pages/settings/InitialSettingsPage.tsx

Line 288 in 935df86

<View style={[menuItemsData.sectionStyle, styles.pb4, styles.mh3]}>

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

We can move scrollview padding style styles.pt4 to contentContainerStyle instead of style.

What alternative solutions did you explore? (Optional)

[shawnborton]

Yeah, I agree with Danny. I guess we could use the same amount of padding we have on the left/right to use at the bottom?

[shawnborton]

I would bring the payout down to $250 or even $125 though, I think this is going to be a very simple fix.

[melvin-bot]

Upwork job price has been updated to $125

[trjExpensify]

Sounds good!

[melvin-bot]

@eVoloshchak, @sonialiap Eep! 4 days overdue now. Issues have feelings too...

[trjExpensify]

Eugene, what's your ETA on reviewing these proposals? Thanks!

[sonialiap]

@eVoloshchak what do you think of the above proposals?

[eVoloshchak]

This seems a regression from #38274 which added style styles.pt4 to scrollview that move the scrollview content bottom with this padding value, I think the style should set to contentContainerStyle instead of style

That is correct, thanks for linking that PR @ahmedGaber93!

All proposals use similar approach, with the first one coming from @Krishna2323
The proposed approach looks good to me

🎀👀🎀 C+ reviewed!

[melvin-bot]

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

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[Krishna2323]

@eVoloshchak PR ready for review.

[Krishna2323]

@sonialiap, PR was deployed to production on 24th April, this is ready for the payments process.

[sonialiap]

Thanks for the ping! Looks like Melvin's automation didn't run 🙈

Payment summary:

• @Krishna2323 $125 paid ✔️
• @eVoloshchak $125 - please request in ND

[JmillsExpensify]

$125 approved for @eVoloshchak