[$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?

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/93399543/50bee367-7157-41a6-af37-2ff6f25a9dcd

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-02

Issue Owner

Current Issue Owner: @fedirjh

[github-actions[bot]]

:wave: 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[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[bot]]

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

[melvin-bot[bot]]

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

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

https://github.com/Expensify/App/blob/80dc8cb62a85374bdde1b7ef005a991dc0720dfb/src/pages/workspace/WorkspaceOverviewPage.js#L77-L108

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)}

https://github.com/Expensify/App/blob/039b57556654eb635662ac1ed1d71ee5b559c167/src/components/AvatarWithImagePicker.js#L336

---

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);
                            }}

https://github.com/Expensify/App/blob/039b57556654eb635662ac1ed1d71ee5b559c167/src/components/AvatarWithImagePicker.js#L333

---

Update Tooltip text here and show Edit photo or View photo

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

https://github.com/Expensify/App/blob/039b57556654eb635662ac1ed1d71ee5b559c167/src/components/AvatarWithImagePicker.js#L330

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.

https://github.com/Expensify/App/blob/3ab4e6e12d9ff38e69493321f0d31f032481b373/src/pages/workspace/WorkspaceOverviewPage.js#L63

This is only true if the isPolicyAdmin is true.

https://github.com/Expensify/App/blob/80dc8cb62a85374bdde1b7ef005a991dc0720dfb/src/pages/workspace/WorkspaceOverviewPage.js#L106

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

https://github.com/Expensify/App/blob/3ab4e6e12d9ff38e69493321f0d31f032481b373/src/components/AvatarWithImagePicker.js#L328-L337

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

[mountiny]

@fedirjh can you please review the proposals? I guess this should work the same as seeing avatars of other users enlarged. Thank you!

[hayata-suenaga]

I guess this should work the same as seeing avatars of other users enlarged.

I agree with with Vit 👍

[bondydaa]

@fedirjh just let me (or vit or hayata) know once you've reviewed the proposals 🙏 🙇

[fedirjh]

@ZhenjaHorbach, your proposal is unclear to me. The disabled prop is used to determine whether the picker is enabled. I don't believe any changes should be made to that. Could you please clarify why you suggest removing disabled?

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

I suggest renaming this prop to something more descriptive, such as enablePreview.

But for this we need to get list of events from AttachmentModal (For this we can use useRef and useImperativeHandle inside AttachmentModal)

Alternatively, wouldn't it be simpler to utilize navigation? We can navigate to the attachment modal, and the workspace avatar is located at https://dev.new.expensify.com:8082/workspace/ID/avatar.

Update Tooltip text here and show Edit photo or View photo

I don't think we should enable the tooltip in this case.

---

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

@Tony-MK, your proposal seems reasonable; however, getWorkspaceIcon returns the associated workspace icon for a given report, and there is no report data within WorkspaceOverviewPage.js. I am unsure if this approach will work or not. Could you please create a proof of concept (POC) for this?

[ZhenjaHorbach]

@ZhenjaHorbach, your proposal is unclear to me. The disabled prop is used to determine whether the picker is enabled. I don't believe any changes should be made to that. Could you please clarify why you suggest removing disabled?

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

I suggest renaming this prop to something more descriptive, such as enablePreview.

But for this we need to get list of events from AttachmentModal (For this we can use useRef and useImperativeHandle inside AttachmentModal)

Alternatively, wouldn't it be simpler to utilize navigation? We can navigate to the attachment modal, and the workspace avatar is located at https://dev.new.expensify.com:8082/workspace/ID/avatar.

Update Tooltip text here and show Edit photo or View photo

I don't think we should enable the tooltip in this case.

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

@Tony-MK, your proposal seems reasonable; however, getWorkspaceIcon returns the associated workspace icon for a given report, and there is no report data within WorkspaceOverviewPage.js. I am unsure if this approach will work or not. Could you please create a proof of concept (POC) for this?

I updated the proposition )

And these were thoughts out loud Disabled does not need to be changed because these changes seem redundant )

[Tony-MK]

Hey @fedirjh, thanks for reviewing my proposal.

Due styling inconsistencies, I changed my approach and optimized the code a bit but you can compare the changes at test the branch

Here is the POC you requested. The second POC is long because it shows the admin inviting the non-admin.

https://github.com/Expensify/App/assets/22982173/8a7da3b6-62cc-4879-8aa1-7965b50ab106

https://github.com/Expensify/App/assets/22982173/aa6246fb-3664-463f-8ead-f6b1a5d7968b

[melvin-bot[bot]]

@bondydaa, @fedirjh, @kadiealexander Whoops! This issue is 2 days overdue. Let's get this updated quick!

[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? 💸

[fedirjh]

@Tony-MK Could you please update your proposal to reflect the changes in https://github.com/Expensify/App/issues/35731#issuecomment-1928815936, also it seems WorkspaceOverviewPage.js was renamed.

[fedirjh]

@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

[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

https://github.com/Expensify/App/blob/310941b513d67480e0f24d69e3adcd3336e8f4b9/src/pages/settings/InitialSettingsPage.js#L343

https://github.com/Expensify/App/blob/30f8e5da79825d1aabb5a4f725739021937fff45/src/pages/workspace/WorkspaceProfilePage.js#L78

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