[HOLD for payment 2023-08-21] [HOLD for #23481] [$1000] mWeb - Sign out - Button hidden under bottom, hard to sign out

[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!

---

Action Performed:

1. launch App and login
2. Tap on profile > Settings
3. Tap on Sign Out

Expected Result:

Sign out button should be visible.

Actual Result:

Button hidden under bottom, hard to sign out

Workaround:

Unknown

Platforms:

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

• [ ] Android / native
• [ ] Android / Chrome
• [ ] iOS / native
• [x] iOS / Safari
• [ ] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.3.27.2

Reproducible in staging?: yes

Reproducible in production?: yes

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/93399543/83ea3c5a-60b4-4af5-bd44-a891b5e3b59f

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f559d55619af61e2
• Upwork Job ID: 1675953147854782464
• Last Price Increase: 2023-07-19

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Bug0 Triage Checklist (Main S/O)

• [ ] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [ ] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• [ ] This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• [ ] This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• [ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[kevinksullivan]

can't reproduce taking it to slack

https://expensify.slack.com/archives/C049HHMV9SM/p1686757919637879

[melvin-bot[bot]]

@kevinksullivan Eep! 4 days overdue now. Issues have feelings too...

[kevinksullivan]

bumping slack thread and closing for now

[kbecciv]

Hello @kevinksullivan I'm reopening the issue, because one of the contributor has the same issue with hidden buttons around the app. Here is the Slack conversation https://expensify.slack.com/archives/C049HHMV9SM/p1686663005982659

https://github.com/Expensify/App/assets/93399543/ff1b3f71-7b89-46d1-966c-0cad02ee5680

https://github.com/Expensify/App/assets/93399543/ec359e7c-905b-4b01-8570-07964012ac17

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01f559d55619af61e2

[melvin-bot[bot]]

Current assignee @kevinksullivan is eligible for the External assigner, not assigning anyone new.

[melvin-bot[bot]]

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

[MaciejSWM]

Hey! I’m Maciej from Software Mansion, an expert agency, and I’d like to investigate this issue.

Anyone managed to reproduce this? I tested on staging, on iOS+Safari, Android+Chrome, iOS simulator and I can't reproduce the issue. I'm opening the app, scrolling, going to workspaces - bottom button looks well placed for all my devices.

[s77rt]

@MaciejSWM I can't reproduce either. Maybe this is only reproducible if the deeplink banner is shown?

[melvin-bot[bot]]

@s77rt @kevinksullivan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[s77rt]

Melvin, this was reopened just yesterday

[kevinksullivan]

Maybe this is only reproducible if the deeplink banner is shown?

@s77rt were you able to reproduce it for this scenario?

[s77rt]

@kevinksullivan I didn't test for that scenario. The banner was not visible in my case, it seems coming from iOS and not the App and only if certain conditions are met(?). I asked in Slack https://expensify.slack.com/archives/C049HHMV9SM/p1688664348845969?thread_ts=1686663005.982659&cid=C049HHMV9SM for more info.

[melvin-bot[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[bot]]

@s77rt @kevinksullivan this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

Current assignee @s77rt is eligible for the Internal assigner, not assigning anyone new.

[melvin-bot[bot]]

@s77rt, @kevinksullivan Huh... This is 4 days overdue. Who can take care of this?

[jscardona12]

Hi @s77rt, @kevinksullivan, i was taking a look into it. i now why is the error being originated. I'm still looking for the best solution.

[melvin-bot[bot]]

📣 @jscardona12! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:

   Contributor details
   Your Expensify account email: <REPLACE EMAIL HERE>
   Upwork Profile Link: <REPLACE LINK HERE>

[jscardona12]

Contributor details Your Expensify account email: js.cardona64@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~014c57c55cdc6e386f

[melvin-bot[bot]]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[jscardona12]

Proposal

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

[mWeb - Sign out - Button hidden under bottom, hard to sign out

What is the root cause of that problem?

The root of the problem is the Open App Banner that detects that you ca use a deeplinnk and shows the app banner. this messes with Safari DOM and makes that the page renders in a wrog way

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

Every time that the settings page is render reload the component for it to take the right measures.

What alternative solutions did you explore? (Optional)

[s77rt]

@jscardona12 Thanks for the proposal. I think your RCA makes sense although can't really validate since I can't reproduce. The solution does not seem the best we can do here. Can you explain how can we detect if the banner is shown (or would be shown)? And if so can we recalculate the height correctly in the first place without the need to have another render as that may cause visual jump behaviour.

[s77rt]

@kevinksullivan Let's make this External again.

[jscardona12]

Hi @s77rt , currently is not possible to detect if the Banner is active, but it is possible to refresh the page without the jump effect. i just did the test and its working correctly.

[jscardona12]

@s77rt

Without the solution:

https://github.com/Expensify/App/assets/16781411/b7c0df3e-fd72-45b8-b025-57b4cc7bfe04

With Solution:

https://github.com/Expensify/App/assets/16781411/e130cf00-62f5-46b5-a870-e0a0f9f68d8f

[s77rt]

@jscardona12 Can you explain on what conditions we will refresh the page?

[jscardona12]

@s77rt The error just happens when you are navigating from the main page. So the page will only refresh when you are navigating from the main page, since there is not a way to detect de app banner in a consistent way on all the devices.

[s77rt]

@jscardona12 Do you mean we will always refresh the page?

[jscardona12]

@s77rt Just when it detects navigations from the main page. if you entered the settings url it wont have a refresh. but the most common use is navigating from the main page so mostly all the times it will refresh the component. Not the page. That is important. The thing reloaded is just the component not all the page.

[s77rt]

@jscardona12 I think we should investigate more on this. Why remounting the components will fix the issue? or what exact component depends on the window height?

[jscardona12]

@s77rt because as you can see in the videos i send, where i show that the solution worked, the main component doesn't have the App Banner, when we enter in the settings page the banner appears but the ScrollView Component still takes the height of the previous page, but because of the Banner that has been reduce and the scroll fails. Remounting the component makes the ScrollView to takethe correct height and the scroll works again. I dont now if you want to have a quick meeting to clarify some doubts.

[s77rt]

@jscardona12 It's still not clear what component we are talking about here. Can you link to that?

[jscardona12]

@s77rt yeah, this is the component ScrollView Inside InitialSettigsPage InitialSettingsPage

[s77rt]

@jscardona12 If that's the case, I think it would be best to fix that on ScrollView.

[jscardona12]

@s77rt ScrollView is a react-native-web component. Should we make an override o the library, or make a Wrapper component to fix this behaviour?

[s77rt]

@jscardona12 We should submit a PR with the fix. I'm still not sure but based on your proposal it appears that ScrollView uses some window/viewport height at mount time but when that height changes (banner is added to the page) the ScrollView would be still using the old incorrect height.

[jscardona12]

Hi @s77rt https://github.com/Expensify/App/pull/22854 this is a draft PR. To sum up, after analizing i figure that we needed to reload the component after the API Called ended. so i implemented a callback to know exactly when to reload the ScrollView component.

[s77rt]

@jscardona12 Please do not raise PRs until you are assigned. The solution looks hacky. If the bug is on ScrollView we should fix that on RNW.

[jscardona12]

Hi @s77rt just did another solution, is in this commit, https://github.com/jscardona12/ExpensifyJuanC/commit/892efc1aff2bce8a3ff5afb32624d73254cfa87f.

Found that the root cause, is the navigator screen for the RightModalNavigator, it is the component that does not update when refreshing. The Height bedore was 100% i put hardcoded but it stills sets 746px instead of 700px that is set. this wasn not updating because of the min-height 100% set on the component. The fix was set the windowHeight to the navigation Screen and also set the minHeight to the same value. altough the value could be 0 as well.

[s77rt]

@jscardona12 Thanks for the update. I don't think your RCA is correct/complete, it does not explain how this is related to the banner.

Please do not post commits, PRs, code diffs.

[jscardona12]

@s77rt The RCA is releated to the banner becasuse the banner is the one that makes the window size to change, to error is the current styles with height 100% does not take the changes. so the proposed fix changes that 100% fot the actual innerHeight and overrides de min-height: 100% to innerHeight as well.

[ginsuma]

@jscardona12 How can you reprocedure it? For me, the banner always show from splash screen. When navigating to other pages, it shows well too.

[s77rt]

Discussing in https://expensify.slack.com/archives/C01GTK53T8Q/p1689516259835559

[jscardona12]

Proposal

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

[mWeb - Sign out - Button hidden under bottom, hard to sign out

What is the root cause of that problem?

The root of the problem is the Open App Banner that detects that you ca use a deeplinnk and shows the app banner. this messes with Safari DOM and makes that the page renders in a wrong way.

This error was previously fix on this PR [PR with previous fix](https://github.com/Expensify/App/pull/19682 but a recent navigation refactor overwrote the changes and didn't took into accout to send the window height to the refactor styles [PR with refactor] (https://github.com/Expensify/App/pull/19263.)

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

pass the windowHeight to the new navigationModalCard implementation in App/src/styles /styles.js. , and apply it to the height property.

navigationModalCard: (isSmallScreenWidth, windowHeight) => ({
    position: 'absolute',
    top: 0,
    right: 0,
    width: isSmallScreenWidth ? '100%' : variables.sideBarWidth,
    backgroundColor: 'transparent',
    height: windowHeight,
}),

This would be re-applying the fix on this [PR with previous fix](https://github.com/Expensify/App/pull/19682 to the new implementation.

[s77rt]

@jscardona12 Thanks for the update. That looks good to me.

:ribbon: :eyes: :ribbon: C+ reviewed Link to proposal

[melvin-bot[bot]]

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

[s77rt]

@kevinksullivan Please mark this as External.