[HOLD for payment 2024-04-02] [$500] IOS- Extra margin below offline indicator in Profile page

[kbecciv]

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.52-0
Reproducible in staging?: y
Reproducible in production?: n
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch New Expensify.
2. Go offline.
3. Go to Profile from the bottom navigation.

Expected Result:

There is no extra margin below offline indicator.

Actual Result:

There is extra margin below offline indicator.

Workaround:

n/a

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0199282bcbfa5f2d37
• Upwork Job ID: 1768345821971177472
• Last Price Increase: 2024-03-14

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @yuwenmemon (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[ahmedGaber93]

Proposal

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

IOS- Extra margin below offline indicator in Profile page

What is the root cause of that problem?

we pass includeSafeAreaPaddingBottom = false but recording to this comment below the ScreenWrapper put paddingbottom equal SafeAreaPaddingBottom even if includeSafeAreaPaddingBottom = false
note SafeAreaPaddingBottom in ios equal the ios notch homebar height and mostly equal 0 in android and other platform

App/src/components/ScreenWrapper.tsx

Lines 223 to 226 in 593d6c0

	// We always need the safe area padding bottom if we're showing the offline indicator since it is bottom-docked.
	if (includeSafeAreaPaddingBottom || (isOffline && shouldShowOfflineIndicator)) {
	paddingStyle.paddingBottom = paddingBottom;
	}

and in this case we should pass pass style styles.pb0 in , but we missed it

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

pass style styles.pb0 in <ScreenWrapper> in InitialSettingsPage here

we can also check other pages if need the same fix

What alternative solutions did you explore? (Optional)

[yuwenmemon]

@ahmedGaber93 does that explain why we're only seeing it on iOS though?

[ahmedGaber93]

@yuwenmemon

does that explain why we're only seeing it on iOS though?

i update the root cause with the explanation.

[yuwenmemon]

Okay, my local iOS is taking forever to build so I'll at least take a look at what you're proposing @ahmedGaber93 - can you spin up a PR with screenshots on all platforms ASAP?

[melvin-bot]

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

[ghost]

I tried @ahmedGaber93 proposal. But is this what we need ? Becoz I think this will make offline message to be very small. @yuwenmemon ?

[ahmedGaber93]

can you spin up a PR with screenshots on all platforms ASAP?

@yuwenmemon yes i can.

[yuwenmemon]

@godofoutcasts94 I think that would be consistent with what I see for other pages:

[ahmedGaber93]

also offline message padding bottom and padding top is consistent now.

[allgandalf]

Proposal

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

Extra margin below offline indicator in Profile page

What is the root cause of that problem?

We add more padding if the device is offline:

App/src/components/ScreenWrapper.tsx

Lines 223 to 226 in 593d6c0

	// We always need the safe area padding bottom if we're showing the offline indicator since it is bottom-docked.
	if (includeSafeAreaPaddingBottom || (isOffline && shouldShowOfflineIndicator)) {
	paddingStyle.paddingBottom = paddingBottom;
	}

For android web and desktop this value is almost 0 but for native iOS this exits (Thanks to @ahmedGaber93, i researched about this after he mentioned it in his proposal)

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

Remove the (isOffline && shouldShowOfflineIndicator) check from the condition, this will help us to maintain consistency among all pages because we use ScreenWrapper at a lot of pages and i have seen that we pass includeSafeAreaPaddingBottom as false in all of them :)

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0199282bcbfa5f2d37

[melvin-bot]

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

[ahmedGaber93]

@yuwenmemon this all screenshots for all platforms.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

[allgandalf]

@yuwenmemon @ishpaul777 , can you review this proposal once, i think fixing it on all pages might be a priority here :)

[ishpaul777]

I'll review proposal in few minutes 🙇‍♂️

[ahmedGaber93]

@GandalfGwaihir we need this lines for other screen to force safeareapaddingbottom when offline

Screen.Recording.2024-03-14.at.8.45.43.PM.mov

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.52-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [CP Staging] fix profile screen OfflineIndicator has extra margin ios #38343

If no regressions arise, payment will be issued on 2024-03-22. 🎊

For reference, here are some details about the assignees on this issue:

• @ahmedGaber93 requires payment (Needs manual offer from BZ)
• @ishpaul777 requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@ishpaul777] The PR that introduced the bug has been identified. Link to the PR:
• [@ishpaul777] 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:
• [@ishpaul777] 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:
• [@ishpaul777] Determine if we should create a regression test for this bug.
• [@ishpaul777] 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.
• [@alexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.53-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [CP Staging] fix profile screen OfflineIndicator has extra margin ios #38343

If no regressions arise, payment will be issued on 2024-03-25. 🎊

For reference, here are some details about the assignees on this issue:

• @ahmedGaber93 requires payment (Needs manual offer from BZ)
• @ishpaul777 requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@ishpaul777] The PR that introduced the bug has been identified. Link to the PR:
• [@ishpaul777] 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:
• [@ishpaul777] 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:
• [@ishpaul777] Determine if we should create a regression test for this bug.
• [@ishpaul777] 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.
• [@alexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[alexpensify]

Here is the payment summary:

• External issue reporter - N/A
• Contributor that fixed the issue - @ahmedGaber93 $500
• Contributor+ that helped on the issue and/or PR - @ishpaul777 $500

Upwork Job: https://www.upwork.com/jobs/~0199282bcbfa5f2d37

Extra Notes regarding payment: @ahmedGaber93 and @ishpaul777 can you please accept the offers in Upwork? After that, I can complete the required process? Thanks!

[ahmedGaber93]

When I am clicking "Accept offer", it failed with this error.
Other offers work fine with me, and I already accept another offer today without this error.

@alexpensify any idea about this error?

[alexpensify]

@ahmedGaber93 - the issue is on the Upwork side, but thank you for sharing. Sorry for the extra step, but I sent over a new offer based on the feedback in the response error.

@ishpaul777 and @ahmedGaber93 - Please try to accept again and I can complete the payment process in Upwork.

[ahmedGaber93]

Offer accepted, thanks @alexpensify

[alexpensify]

I've paid @ahmedGaber93 via Upwork. I'll need to keep this one open because I'm waiting for @ishpaul777 to accept the new proposal.

[ishpaul777]

@alexpensify Can we please hold payment till 2 April, is it possible?

[alexpensify]

@ishpaul777 - yes, but can you please share a little more context?

[ishpaul777]

No big deal just becuase i have applied for GSTIN which upwork wants me to submit otherwise i got charged extra tax.
https://support.upwork.com/hc/en-us/articles/360041114713-India-GST-for-freelancers

[alexpensify]

Thanks, I'll set a reminder to check on April 2. Please keep me posted if I need to change this payment date.

[alexpensify]

Updated per the request here.

[ishpaul777]

Thank you! 🤗

[alexpensify]

@ishpaul777 - can you please share an update if your Upwork profile is ready for this payment? If yes, I'll work on the required steps tomorrow morning. Thanks!

[ishpaul777]

Thanks for holding @alexpensify! Can you please send in the offer https://www.upwork.com/freelancers/~01f4fddf5caa8bfc3a

[alexpensify]

All set here, I've paid everyone here via Upwork. Summary:

#38328 (comment)