mWeb/Safari - Group chat - Photo upload options not displayed when tapped on upload photo menu

[IuliiaHerets]

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: v9.0.24-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4887343
Email or phone of affected tester (no customers): applausetester+ck822@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

Preconditions:

1. User has created new account
2. User has created group chat with few users with custom avatar & group name

Steps:

1. Navigate to staging.new.expensify.com
2. Open the created group chat
3. Click on the header of the conversation
4. Click on the avatar
5. Click on Upload photo menu

Expected Result:

Photo upload options (Photo Library, take photo & Choose file) should open & user able to change group avatar

Actual Result:

Photo upload options not displayed when tapped on upload photo menu in group chat details page

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6579763_1724366077675.RPReplay_Final1724366004.mp4

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @slafortune (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.

[melvin-bot]

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

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

[IuliiaHerets]

Production - works well

RPReplay_Final1724389159.MP4

[ChavdaSachin]

This issue is similar to This Issue and was introduced after This pr.

[techievivek]

I see that a revert PR is already in progress. Let me check with the team to see if we can proceed with the merge and do a cherry-pick here.

[fedirjh]

cc @techievivek I think I have a quick fix for both bugs instead of just reverting the PR.

[fedirjh]

What is the root cause of that problem?

On mobile Safari, The file picker modal is opened with a different approach that other platforms. In this line, we have a check for Browser.isSafari() if the browser is Safari, it will just return :

App/src/components/AvatarWithImagePicker.tsx

Lines 268 to 270 in afeb37e

	if (Browser.isSafari()) {
	return;
	}

In this block, we check if the Browser.isSafari(), then we open the file modal selector accordingly.

App/src/components/AvatarWithImagePicker.tsx

Lines 411 to 418 in afeb37e

	// In order for the file picker to open dynamically, the click
	// function must be called from within an event handler that was initiated
	// by the user on Safari.
	if (index === 0 && Browser.isSafari()) {
	openPicker({
	onPicked: showAvatarCropModal,
	});
	}

However, with the new implemented prop shouldCloseAllModals , all modals will be closed when the onSelected callback is called.

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

Inside AttachmentPickerWithMenuItemsProps and AvatarWithImagePicker, we should default shouldCloseAllModals to !Browser.isSafari() instead of true ,

-     shouldCloseAllModals={true}
+     shouldCloseAllModals={!Browser.isSafari()}

@techievivek Let me know if that makes sense and I can raise a quick PR accordingly

[fedirjh]

cc @techievivek PR is ready: #47907

[techievivek]

That looks promising, thanks for taking a look.

[techievivek]

Let me discuss with @arosiclair to see what would be best here.

[arosiclair]

Fixed by revert

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

⚠️ 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.