[HOLD for payment 2024-07-17] [$250] Public room - Two RHPs appear briefly after clicking on any option in group details RHP

[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: 1.4.84-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
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Log out if logged in
3. Go to a public room
4. Click on the chat header
5. Click any option (Invite, Settings, Private notes)

Expected Result:

Sign in modal will animate smoothly

Actual Result:

Two RHPs appear briefly after clicking on any option in group details RHP

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

public.room.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d9bd9f1e714cb88f
• Upwork Job ID: 1803182365680551956
• Last Price Increase: 2024-06-18
• Automatic offers:
• DylanDylann | Reviewer | 102792321
• bernhardoj | Contributor | 102792322

Issue Owner

Current Issue Owner: @johncschuster

[melvin-bot]

Triggered auto assignment to @johncschuster (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]

@johncschuster 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

[lanitochka17]

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

[bernhardoj]

Proposal

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

Two RHPs briefly appear when open the sign in modal from an RHP as anon user.

What is the root cause of that problem?

This issue has happened before in #28610. The root cause is the same. In login form, we auto focus to the field which interrupts the screen animation. We previously fixed it by adding a delay just like on other pages,

App/src/pages/signin/LoginForm/BaseLoginForm.tsx

Lines 170 to 175 in 75c104b

	let focusTimeout: NodeJS.Timeout;
	if (isInNarrowPaneModal) {
	focusTimeout = setTimeout(() => input.current?.focus(), CONST.ANIMATED_TRANSITION);
	} else {
	input.current.focus();
	}

but the auto focus is still happened immediately. It happens after this PR which adds the autoFocus prop to the text input. It's to fix this issue where the focus trap initial focus cause the issue.

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

First, remove the autoFocus prop from the text input.

App/src/pages/signin/LoginForm/BaseLoginForm.tsx

Line 271 in 75c104b

autoFocus

To fix #43664, add the sign in modal screen (SCREENS.SIGN_IN_ROOT) to the SCREENS_WITH_AUTOFOCUS list.

App/src/components/FocusTrap/SCREENS_WITH_AUTOFOCUS.ts

Lines 4 to 13 in 75c104b

	const SCREENS_WITH_AUTOFOCUS: string[] = [
	...Object.keys(CENTRAL_PANE_WORKSPACE_SCREENS),
	SCREENS.REPORT,
	SCREENS.REPORT_DESCRIPTION_ROOT,
	SCREENS.PRIVATE_NOTES.EDIT,
	SCREENS.SETTINGS.PROFILE.STATUS,
	SCREENS.SETTINGS.PROFILE.PRONOUNS,
	SCREENS.NEW_TASK.DETAILS,
	SCREENS.MONEY_REQUEST.CREATE,
	];

This perfectly fits our needs because we don't want the focus trap initial focus override the page auto focus.

App/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx

Lines 50 to 55 in 75c104b

	// We don't want to ovverride autofocus on these screens.
	initialFocus: () => {
	if (SCREENS_WITH_AUTOFOCUS.includes(activeRouteName)) {
	return false;
	}
	return undefined;

[melvin-bot]

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

[melvin-bot]

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

[DylanDylann]

@bernhardoj's proposal looks good to me

The autoFocus prop and the initial focus from focus trap implementation are unnecessary on this page. Let'e remove it

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @bernhardoj 🎉 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 📖

[bernhardoj]

PR is ready

cc: @DylanDylann

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Fix RHP slide animation when opening sign in modal #43985

If no regressions arise, payment will be issued on 2024-07-17. 🎊

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

• @bernhardoj requires payment automatic offer (Contributor)
• @DylanDylann requires payment automatic offer (Reviewer)

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

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

[bernhardoj]

I'll request in ND once payment is due.

[johncschuster]

Payment Summary:

Contributor: @bernhardoj - $250 - via Manual Request
Contributor+: @DylanDylann - $250 - via Upwork

[melvin-bot]

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

• Fix RHP slide animation when opening sign in modal #43985

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

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

• @bernhardoj requires payment automatic offer (Contributor)
• @DylanDylann requires payment automatic offer (Reviewer)

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

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

[bernhardoj]

The payment should be due tomorrow (07-17)

[johncschuster]

Payment Summary:

Contributor: @bernhardoj owed $250 via NewDot
Contributor+: @DylanDylann paid $250 via Upwork

[bernhardoj]

Requested in ND.

[johncschuster]

I've issued the payment in Upwork. Jason will take care of NewDot payment.

@DylanDylann can you please post a regression test list if you feel we need one?

[JmillsExpensify]

$250 approved for @bernhardoj

[DylanDylann]

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:

[@DylanDylann] The PR that introduced the bug has been identified. Link to the PR: #43752
[@DylanDylann] 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: #43752 (comment)
[@DylanDylann] 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: NA
[@DylanDylann] Determine if we should create a regression test for this bug. No
[@DylanDylann] 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.

Do we agree 👍 or 👎