[HOLD for payment 2024-02-26] [$500] Workspace - Clicking on avatar of invited workspace does not open avatar in full screen view

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.36.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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Precondition:

• User is invited to a workspace that has custom avatar.

1. Go to staging.new.expensify.com.
2. Go to settings from the bottom.
3. Click on the invited workspace.
4. Click on the workspace avatar.

Expected Result:

Workspace avatar will open in full screen view.

Actual Result:

Nothing happens when clicking on the avatar of invited workspace. Clicking on avatar of invited workspace does not open avatar in full screen view.

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

Bug6365315_1706907042565.20240203_005005.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0188ba50d52f570fef
• Upwork Job ID: 1753549469140381696
• Last Price Increase: 2024-02-09
• Automatic offers:
• fedirjh | Reviewer | 0
• ZhenjaHorbach | Contributor | 0

Issue Owner

Current Issue Owner: @fedirjh

[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 @bondydaa (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[kbecciv]

We think that this bug might be related to #wave5-free-submitters
CC @dylanexpensify

[bondydaa]

Asked about this here https://expensify.slack.com/archives/C05DWUDHVK7/p1706912992107429

I don't think this is a bug or a blocker because we didn't really consider it during implementation... that being said, I do think for the read-only view, that an employee should be able to see the avatar at a larger size in the attachment modal

removing the blocker and going to put this out for external proposals.

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0188ba50d52f570fef

[melvin-bot]

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

[bondydaa]

@kadiealexander no action required from ya for now.

[bondydaa]

what we're looking for is a way to view workspace avatars in the bigger "attachment modal" when following these steps:

Go to settings from the bottom.
Click on the invited workspace.
Click on the workspace avatar (currently does nothing, should open image in modal)

[ZhenjaHorbach]

Proposal

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

Workspace - Clicking on avatar of invited workspace does not open avatar in full screen view

What is the root cause of that problem?

The main problem with the issue is that we have disabled param which disables any interaction with the image

App/src/pages/workspace/WorkspaceOverviewPage.js

Lines 77 to 108 in 80dc8cb

	<AvatarWithImagePicker
	onViewPhotoPress={() => Navigation.navigate(ROUTES.WORKSPACE_AVATAR.getRoute(policy.id))}
	source={lodashGet(policy, 'avatar')}
	size={CONST.AVATAR_SIZE.XLARGE}
	avatarStyle={styles.avatarXLarge}
	DefaultAvatar={() => (
	<Avatar
	containerStyles={styles.avatarXLarge}
	imageStyles={[styles.avatarXLarge, styles.alignSelfCenter]}
	source={policy.avatar ? policy.avatar : ReportUtils.getDefaultWorkspaceAvatar(policyName)}
	fallbackIcon={Expensicons.FallbackWorkspaceAvatar}
	size={CONST.AVATAR_SIZE.XLARGE}
	name={policyName}
	type={CONST.ICON_TYPE_WORKSPACE}
	/>
	)}
	type={CONST.ICON_TYPE_WORKSPACE}
	fallbackIcon={Expensicons.FallbackWorkspaceAvatar}
	style={[styles.mb3, styles.mt5]}
	isUsingDefaultAvatar={!lodashGet(policy, 'avatar', null)}
	onImageSelected={(file) => Policy.updateWorkspaceAvatar(lodashGet(policy, 'id', ''), file)}
	onImageRemoved={() => Policy.deleteWorkspaceAvatar(lodashGet(policy, 'id', ''))}
	editorMaskImage={Expensicons.ImageCropSquareMask}
	pendingAction={lodashGet(policy, 'pendingFields.avatar', null)}
	errors={lodashGet(policy, 'errorFields.avatar', null)}
	onErrorClose={() => Policy.clearAvatarErrors(policy.id)}
	previewSource={UserUtils.getFullSizeAvatar(policy.avatar, '')}
	headerTitle={translate('workspace.common.workspaceAvatar')}
	originalFileName={policy.originalFileName}
	disabled={readOnly}
	disabledStyle={styles.cursorDefault}
	/>

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

To fix this issue we can add a new property (enablePreview)

To add the ability to press on image we can make changes here

const readOnly = enablePreview && !isUsingDefaultAvatar;
disabled={isAvatarCropModalOpen || (disabled && !readOnly)}

App/src/components/AvatarWithImagePicker.js

Line 336 in 039b575

disabled={isAvatarCropModalOpen || disabled}

---

Add a condition here so that instead of opening the menu
We opened the screen with the photo

For this, we can use onViewPhotoPress from the parent component which we use to navigate to a modal window with an image

Additional ways

> We can make `AttachmentModal` like wrapper for all image component to use show function from `AttachmentModal`

We can get list of events from AttachmentModal (For this we can use useRef and useImperativeHandle inside AttachmentModal)

                            onPress={() => {
                                if (disabled && readOnly) {
                                    onViewPhotoPress();
                                    return;
                                }
                                setIsMenuVisible((prev) => !prev);
                            }}

App/src/components/AvatarWithImagePicker.js

Line 333 in 039b575

onPress={() => setIsMenuVisible((prev) => !prev)}

---

Update Tooltip text here and show Edit photo or View photo

text={readOnly ? translate('avatarWithImagePicker.viewPhoto') : translate('avatarWithImagePicker.editImage')}

App/src/components/AvatarWithImagePicker.js

Line 330 in 039b575

text={translate('avatarWithImagePicker.editImage')}

Plus I notice that the close button on AttachmentModal doesn't have a selected cursor
We can fix this too

What alternative solutions did you explore? (Optional)

NA

[Tony-MK]

Proposal

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

Workspace - Clicking on avatar of invited workspace does not open avatar in full screen view

What is the root cause of that problem?

In the WorkspaceOverviewPage, the AvatarWithImagePicker component disables its PressableWithoutFeedback component when the user can only read the image and not edit it.

App/src/pages/workspace/WorkspaceOverviewPage.js

Line 63 in 3ab4e6e

const readOnly = !PolicyUtils.isPolicyAdmin(policy);

This is only true if the isPolicyAdmin is true.

App/src/pages/workspace/WorkspaceOverviewPage.js

Line 106 in 80dc8cb

disabled={readOnly}

Hence, if readOnly is true, the onViewPhotoPress function will not be called.

App/src/components/AvatarWithImagePicker.js

Lines 328 to 337 in 3ab4e6e

	<Tooltip
	shouldRender={!disabled}
	text={translate('avatarWithImagePicker.editImage')}
	>
	<PressableWithoutFeedback
	onPress={() => setIsMenuVisible((prev) => !prev)}
	accessibilityRole={CONST.ACCESSIBILITY_ROLE.IMAGEBUTTON}
	accessibilityLabel={translate('avatarWithImagePicker.editImage')}
	disabled={isAvatarCropModalOpen || disabled}
	disabledStyle={disabledStyle}

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

Since we don't need to edit the avatar when the user is not an admin but can view it, I think it is reasonable to consider using the RoomHeaderAvatars component instead of the AvatarWithImagePicker component like the ReportDetailsPage.

We will get the single icon to pass into the icons prop from the getWorkspaceIcon function.

Therefore, we can use a simple condition to either render the AvatarWithImagePicker or RoomHeaderAvatars component should be rendered based on the isPolicyAdmin condition.

[trjExpensify]

I've popped this one in the wave8 project, as it's a byproduct of the change with ideal nav to let non-admin members view the workspace overview page. 👍 CC: @mountiny @hayata-suenaga

[ZhenjaHorbach]

@ZhenjaHorbach Your proposal looks overcomplicated to me. Specifically this part :

Add condition here so that instead of opening the menu We opened the screen with the photo
We have some ways: 1. We need to get list of events from AttachmentModal (For this we can use useRef and useImperativeHandle inside AttachmentModal)
2. We can make AttachmentModal like wrapper for all image component to use show function from AttachmentModal
3. Use onViewPhotoPress from parent component which we use to navigate to a modal window with an image

There are 3 options to choose from
Each of these options works well
But I can cut it

[fedirjh]

@ZhenjaHorbach Reviewing the proposal, I believe we can implement a simpler solution:

1. Introduce a new prop named previewRoute to the AvatarWithImagePicker component.
2. Within the WorkspaceProfilePage, include the prop like this: previewRoute={ROUTES.WORKSPACE_AVATAR.getRoute(policy.id)}
3. Update the press handler to navigate to the previewRoute if it exists and the picker is disabled:

onPress={() => {
    if (previewRoute && disabled) {
        Navigation.navigate(previewRoute);
        return;
    }
    setIsMenuVisible((prev) => !prev);
}}

With this approach, we can ensure that the AvatarWithImagePicker is used for similar use cases. All that's needed is to provide a preview route and disable the picker option.

What do you think @bondydaa?

[ZhenjaHorbach]

@ZhenjaHorbach Reviewing the proposal, I believe we can implement a simpler solution:

1. Introduce a new prop named previewRoute to the AvatarWithImagePicker component.
2. Within the WorkspaceProfilePage, include the prop like this: previewRoute={ROUTES.WORKSPACE_AVATAR.getRoute(policy.id)}
3. Update the press handler to navigate to the previewRoute if it exists and the picker is disabled:

onPress={() => {
    if (previewRoute && disabled) {
        Navigation.navigate(previewRoute);
        return;
    }
    setIsMenuVisible((prev) => !prev);
}}

With this approach, we can ensure that the AvatarWithImagePicker is used for similar use cases. All that's needed is to provide a preview route and disable the picker option.

What do you think @bondydaa?

I think there is no easier way then onViewPhotoPress instead Navigation.navigate )
Because it is responsible for where we will navigate when we click onView Photo

App/src/pages/settings/InitialSettingsPage.js

Line 343 in 310941b

onViewPhotoPress={() => Navigation.navigate(ROUTES.PROFILE_AVATAR.getRoute(accountID))}

App/src/pages/workspace/WorkspaceProfilePage.js

Line 78 in 30f8e5d

onViewPhotoPress={() => Navigation.navigate(ROUTES.WORKSPACE_AVATAR.getRoute(policy.id))}

[fedirjh]

I think there is no easier way then onViewPhotoPress )

This was not so clear to me when I first saw it. Let's reuse that instead of the previewRoute .

[fedirjh]

I think this proposal from @ZhenjaHorbach is good. We should only verify the enablePreview && onViewPhotoPress props; there is no need to check for isUsingDefaultAvatar since the default avatar should be displayed in fullscreen mode, similar to profile avatars.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @bondydaa is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[fedirjh]

@Tony-MK Thanks for the proposal. Your proposal looks good as well, but I think fixing AvatarWithImagePicker is the best solution for this issue.

[melvin-bot]

📣 @fedirjh 🎉 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]

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

[ZhenjaHorbach]

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

PR will be ready today or tomorrow

[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 1.4.42-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix bug with Clicking on avatar of invited workspace does not open avatar in full screen view #36395

If no regressions arise, payment will be issued on 2024-02-26. 🎊

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

• @fedirjh requires payment automatic offer (Reviewer)
• @ZhenjaHorbach requires payment automatic offer (Contributor)

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

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

[fedirjh]

BugZero Checklist:

• Link to the PR: I think this is a feature request.
• Link to comment: N/A
• Link to discussion: N/A
• Determine if we should create a regression test for this bug: N/A, we have a regression test in place.

[kadiealexander]

Payouts due:

• Contributor: $500 @ZhenjaHorbach
• Contributor+: $500 @fedirjh

Upwork job is here.