[Payment due date 7/30][$250] Remove "Upload photo" step for users who are adding a group image for the first time #42868

jamesdeanexpensify · 2024-05-30T20:15:54Z

Coming from this Slack thread, the first time you go to add a picture to a group chat, we add an extra (unnecessary) click/tap on "Upload photo":

You click the camera icon near the group avatar
It brings up "Upload photo"
You click "Upload photo"
It then brings up three options that you can actually do something with (Take photo, Choose from gallery, Choose document)

Recommendation: When it's your first time adding a photo to a group chat, skip the "Upload photo" step and go straight to the step with the three options. Video of how it currently looks below.

RPReplay_Final1717037990.1.mov

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01b0f3bb834f330c4f
Upwork Job ID: 1796274972449415168
Last Price Increase: 2024-06-06
Automatic offers:

b4s36t4 | Contributor | 102899317

melvin-bot · 2024-05-30T20:18:23Z

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

melvin-bot · 2024-05-30T20:18:28Z

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

abzokhattab · 2024-05-30T20:27:34Z

Proposal

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

Remove "Upload photo" step for users who are adding a image for the first time

What is the root cause of that problem?

if the avatar having the default image then it means its the first time , thus we can enabled the 3 dot menu if the default image is shown

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

first we need to keep a ref of the openPicker method in order to use it inside when pressing on the avatar icon

const openPickerRef = useRef<OpenPicker | null>(null);

then inside the attachment picker here we need to assign the openPickerRef

                            {({openPicker}) => {
                                openPickerRef.current = openPicker;

finally inside the onPress of the PressableWithoutFeedback
here we need to add the following condition to be the same as in the createMenuItems

       if (isUsingDefaultAvatar) { 
                                    openPickerRef.current?.({
                                        onPicked: showAvatarCropModal,
                                    });
                                    return;
                                }

to clean the code more we can put the function inside the onSelect of the createMenuItems inside a shared function that should be used in both locations

POC

Screen.Recording.2024-05-30.at.11.23.30.PM.mov

optional

instead of always showing the 3 dot menu, we can prop shouldSkipUploadMenu and set the default value to false
then update this condition aboveif (isUsingDefaultAvatar) to if (isUsingDefaultAvatar && shouldSkipUploadMenu)
then inside the NewChatConfirmPage, we should add this new prop as shouldSkipUploadMenu={!stashedLocalAvatarImage}

Krishna2323 · 2024-05-30T20:42:04Z

Proposal

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

Remove "Upload photo" step for users who are adding a image for the first time

What is the root cause of that problem?

New feat

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

We can accept a new prop in AvatarWithImagePicker to skip the Upload Photo modal and before calling createMenuItems we can check for the prop and run the openPicker. We will pass the prop from NewChatConfirmPage.

if (shouldSkipUploadPhotoPrompt && isMenuVisible) {
    setIsMenuVisible(false);
    openPicker({
        onPicked: showAvatarCropModal,
    });
    return null;
}

Apart from the prop we also need to check for isMenuVisible and update its state otherwise the picker will appear multiple times after cancellation.

App/src/components/AvatarWithImagePicker.tsx

Line 369 in 6e230e9

{({openPicker}) => {

"

We can also use isUsingDefaultAvatar if we need to apply this to all places.

What alternative solutions did you explore? (Optional)

Krishna2323 · 2024-05-30T21:12:53Z

Proposal Updated

Updated code snippet to also check for isMenuVisible and added a note.

abzokhattab · 2024-05-30T21:21:11Z

Proposal updated

fixed the rerendering issue

b4s36t4 · 2024-05-30T21:34:39Z

Proposal

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

Remove "Upload photo" step for users who are adding a group image for the first time

What is the root cause of that problem?

NA - New feature

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

We already have the Attachment Picker setup here

App/src/components/AvatarWithImagePicker.tsx

Line 368 in 29148f6

which handles showing the both modals where we handle both events i.e Upload Photo and three menu options.

We shouldn't distrub the existing implementation as it is wrapped with AttachmentModal which by defaults mounted and as well as AttachmentPicker is also mounted which can cause issues and can call openPicker function everytime we call the function even though we return null.

The optiomal Solution is to updated the AttachmentModal and AttachmentPicker to the whole component and use the openPicker function directly within the onPress function of PressableWithoutFeedback component.

The change follows as below

<View style={styles.w100}>
                <AttachmentModal
                    headerTitle={headerTitle}
                    source={previewSource}
                    originalFileName={originalFileName}
                    fallbackSource={fallbackIcon}
                    maybeIcon={isUsingDefaultAvatar}
                >
                    {({show}) => (
                        <AttachmentPicker type={CONST.ATTACHMENT_PICKER_TYPE.IMAGE}>
                            {({openPicker}) => {
                                const menuItems = createMenuItems(openPicker);

                                // If the current avatar isn't a default avatar and we are not overriding this behavior allow the "View Photo" option
                                if (!shouldDisableViewPhoto && !isUsingDefaultAvatar) {
                                    menuItems.push({
                                        icon: Expensicons.Eye,
                                        text: translate('avatarWithImagePicker.viewPhoto'),
                                        onSelected: () => {
                                            if (typeof onViewPhotoPress !== 'function') {
                                                show();
                                                return;
                                            }
                                            onViewPhotoPress();
                                        },
                                    });
                                }

                                return (
                                    <>
                                        <OfflineWithFeedback
                                            errors={errors}
                                            errorRowStyles={errorRowStyles}
                                            onClose={onErrorClose}
                                        >
                                            <Tooltip
                                                shouldRender={!disabled}
                                                text={translate('avatarWithImagePicker.editImage')}
                                            >
                                                <PressableWithoutFeedback
                                                    onPress={() => {
                                                        if (isUsingDefaultAvatar) {
                                                            openPicker({
                                                                onPicked: showAvatarCropModal,
                                                            });
                                                            return;
                                                        }
                                                        if (disabled && enablePreview && onViewPhotoPress) {
                                                            onViewPhotoPress();
                                                            return;
                                                        }
                                                        setIsMenuVisible((prev) => !prev);
                                                    }}
                                                    accessibilityRole={CONST.ACCESSIBILITY_ROLE.IMAGEBUTTON}
                                                    accessibilityLabel={translate('avatarWithImagePicker.editImage')}
                                                    disabled={isAvatarCropModalOpen || (disabled && !enablePreview)}
                                                    disabledStyle={disabledStyle}
                                                    style={[styles.pRelative, avatarStyle, type === CONST.ICON_TYPE_AVATAR && styles.alignSelfCenter]}
                                                    ref={anchorRef}
                                                >
                                                    <OfflineWithFeedback pendingAction={pendingAction}>
                                                        {source ? (
                                                            <Avatar
                                                                containerStyles={avatarStyle}
                                                                imageStyles={[avatarStyle, styles.alignSelfCenter]}
                                                                source={source}
                                                                avatarID={avatarID}
                                                                fallbackIcon={fallbackIcon}
                                                                size={size}
                                                                type={type}
                                                            />
                                                        ) : (
                                                            <DefaultAvatar />
                                                        )}
                                                    </OfflineWithFeedback>
                                                    {!disabled && (
                                                        <View style={StyleSheet.flatten([styles.smallEditIcon, styles.smallAvatarEditIcon, editIconStyle])}>
                                                            <Icon
                                                                src={editIcon}
                                                                width={variables.iconSizeSmall}
                                                                height={variables.iconSizeSmall}
                                                                fill={theme.icon}
                                                            />
                                                        </View>
                                                    )}
                                                </PressableWithoutFeedback>
                                            </Tooltip>
                                        </OfflineWithFeedback>
                                        <PopoverMenu
                                            isVisible={isMenuVisible}
                                            onClose={() => setIsMenuVisible(false)}
                                            onItemSelected={(item, index) => {
                                                setIsMenuVisible(false);
                                                // 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,
                                                    });
                                                }
                                            }}
                                            menuItems={menuItems}
                                            anchorPosition={shouldUseStyleUtilityForAnchorPosition ? styles.popoverMenuOffset(windowWidth) : popoverPosition}
                                            anchorAlignment={{horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT, vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP}}
                                            withoutOverlay
                                            anchorRef={anchorRef}
                                        />
                                    </>
                                );
                            }}
                        </AttachmentPicker>
                    )}
                </AttachmentModal>
            </View>

What alternative solutions did you explore? (Optional)

NA

Kapture.2024-05-31.at.03.06.08.mp4

melvin-bot · 2024-06-03T19:40:47Z

@mollfpr Whoops! This issue is 2 days overdue. Let's get this updated quick!

tienifr · 2024-06-04T07:42:50Z

Proposal

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

When clicking on avatar, if there is only one option "Upload photo" to select, it still displays. User need to click on that option, then three options are displayed. It is redundant.

What is the root cause of that problem?

In here, when clicking on avatar, we always open the menu ["Upload photo", ...] without considering whether it only contains one option or not.

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

In general, we should not display the menu ["Upload photo", ...] if it contains only one option.
To do it, create a const openMenuRef = useRef<() => void>() in here.
Then in here:

                                openMenuRef.current = () => {
                                    if (menuItems.length === 1) {
                                        openPicker({
                                            onPicked: showAvatarCropModal,
                                        });
                                    } else {
                                        setIsMenuVisible(true);
                                    }
                                };

Finally, call it in here:

                               openMenuRef.current?.();

With this, we always skip the menu ["Upload photo", ...] if it contains only one option.

What alternative solutions did you explore? (Optional)

We can expose the show function in AttachmentModal and openPicker function in AttachmentPicker by using useImperativeHandle.
With this, in AvatarWithImagePicker, we can use two above method everywhere, so the code will be easier to read. Below is the pseudo-code:

    const attachmentModalRef = useRef();

    const attachmentPickerRef = useRef();

    const menuItems = useMemo(()=>{
        const menu = createMenuItems(attachmentPickerRef.current?.openPicker);
        if (!shouldDisableViewPhoto && !isUsingDefaultAvatar) {
          menu.push({
            icon: Expensicons.Eye,
            text: translate('avatarWithImagePicker.viewPhoto'),
            onSelected: () => {
                if (typeof onViewPhotoPress !== 'function') {
                    attachmentModalRef.current?.show();
                    return;
                }
                onViewPhotoPress();
            },
        });
    }
    return menu
    }, [createMenuItems])

    const onItemSelected = (item, index) => {
        setIsMenuVisible(false);
        if (index === 0 && Browser.isSafari()) {
            attachmentPickerRef.current?.openPicker({
                onPicked: showAvatarCropModal,
            });
        }
    };

return <View>
               <PressableWithoutFeedback
                            onPress={() => {
                                if (disabled && enablePreview && onViewPhotoPress) {
                                    onViewPhotoPress();
                                    return;
                                }
                                if(menuItems.length === 1){
                                    attachmentPickerRef.current?.openPicker({
                                        onPicked: showAvatarCropModal,
                                    });
                                }
                                setIsMenuVisible((prev) => !prev);
                            }}
                        >
                </PressableWithoutFeedback>

                <AttachmentModal ref={attachmentModalRef} >
                    <AttachmentPicker ref={attachmentPickerRef} >
                        <PopoverMenu
                            isVisible={isMenuVisible}
                            onItemSelected={onItemSelected}                            
                            menuItems={menuItems}
                            anchorRef={anchorRef}
                        />
                    </AttachmentPicker>
                </AttachmentModal>
        </View>

melvin-bot · 2024-06-04T21:33:16Z

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

melvin-bot · 2024-06-05T18:09:50Z

@mollfpr Huh... This is 4 days overdue. Who can take care of this?

mollfpr · 2024-06-06T15:42:11Z

Will take a look at this in the morning!

melvin-bot · 2024-06-06T16:00:06Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

mollfpr · 2024-06-07T12:40:56Z

The ref function solution seems promising, but I think it will be hard to follow where the ref gonna be used. Considering the ref is used outside the AttachmentPicker. I'll save this for later.

@b4s36t4 I tried your solution, but I got this error when trying to apply it.

@Krishna2323 I got this error Warning: Error: Too many re-renders. React limits the number of renders to prevent an infinite loop. on iOS.

Screen.Recording.2024-06-07.at.19.36.56.mp4

b4s36t4 · 2024-06-07T13:17:12Z

@mollfpr Sorry, I might've missed checking it on web.

The solution for that error is instead of wrapping the AttachmentPicker over the PressableWithoutFeedback please wrap it over the Tooltip.

Updated code as follows.

I'll also update my proposal too.

mollfpr · 2024-06-07T13:49:07Z

@b4s36t4 It's working now. Although, I'm curious if we can just move the AttachmentPicker up to wrap both PressableWithoutFeedback and AttachmentModal, so we don't have to render unnecessary input components and popover while we only use one of them.

b4s36t4 · 2024-06-07T13:50:32Z

Yea I also thought the same, but thought it won't be feasible but I'm open to it.

Do you want me to take a look at that approach as well @mollfpr ?

mollfpr · 2024-06-07T14:02:35Z

@b4s36t4 Yeah, let's try it.

b4s36t4 · 2024-06-07T14:03:28Z

Sure, I'll check and update you soon.

tienifr · 2024-06-07T15:23:36Z

@mollfpr I add the alternative solution that will use expose the show function in AttachmentModal and openPicker function in AttachmentPicker by using useImperativeHandle.

b4s36t4 · 2024-06-07T16:44:47Z

@mollfpr I have tested the solution you suggested, it's working same expected.

mollfpr · 2024-06-07T18:01:51Z

@b4s36t4 could you update your proposal with the new implementation?

@jamesdeanexpensify I think we need to add the BZ label here.

b4s36t4 · 2024-06-07T18:10:00Z

@mollfpr I did updated can you verify again please?

Christinadobrzyn · 2024-06-13T01:53:52Z

Just a heads up - I'm going to be ooo until June 24th. I'm not going to assign anyone new but if you need a new BZ teammate for any reason please feel free to ask for one to be assigned here.

tienifr · 2024-06-13T13:51:23Z

@mollfpr Could you please add your final decision here to help @nkuoch wrap things up more easily and ensure nothing is missed?

mollfpr · 2024-06-13T16:16:32Z

I have checked our component that uses useImperativeHandle. In our practice, we use useImperativeHandle when we don't pass down the function we need from the parent component. In this case, we don't have any problem using the function from the parent component because it is passed down to the children.

As I said, it's good to give attention to the performance issue but currently, we don't block any use of the inline function and it is used all over the place. Also, @tienifr if you can show the measurement for the performance issue between the two solutions that would be a huge consideration.

mollfpr · 2024-06-27T08:50:48Z

Friendly bump @nkuoch

nkuoch · 2024-06-27T12:00:35Z

Let's go with @b4s36t4

melvin-bot · 2024-06-27T12:00:51Z

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

Christinadobrzyn · 2024-07-02T19:40:56Z

update for Melvin - working on PR #44702

Christinadobrzyn · 2024-07-09T15:46:31Z

update for Melvin - working on PR #44702

Christinadobrzyn · 2024-07-16T23:13:51Z

How's the PR coming @mollfpr and @b4s36t4?

mollfpr · 2024-07-17T05:27:27Z

@Christinadobrzyn we waiting for the fix from @b4s36t4. It not much and I can finish the review after the update.

melvin-bot · 2024-07-20T20:17:00Z

⚠️ 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 · 2024-07-20T20:17:32Z

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

Christinadobrzyn · 2024-07-23T23:46:49Z

Just checking in on this - The PR is merged but we might have some deploy blockers?

b4s36t4 · 2024-07-24T03:11:58Z

No, it's not related to this PR. It is deployed to prod as well, but I'm not sure why Melvin didn't give any message here.

b4s36t4 · 2024-07-25T09:17:20Z

@Christinadobrzyn As per this comment #44702 (comment) this pr did hit the product on the 23rd which I feel is the starting day of the regression period.

I don't know maybe because of deploy-blocker messages melvin didn't add the review messages here. Can you update the details here so that it won't be missed?

Thanks!

Christinadobrzyn · 2024-07-30T16:59:56Z

Thanks, @b4s36t4 - I'll add the payment details, as this should be paid today/tomorrow.

Payouts due:

Contributor: $250 @b4s36t4 (Paid in Upwork - https://www.upwork.com/nx/wm/offer/102899317)
Contributor+: $250 @mollfpr (paid through NewDot)

It's a new feature but do we want a regression test for this?

mollfpr · 2024-07-30T17:24:51Z

I'm down with the regression test.

Open the App, click on the (+) plus icon, and then choose Start Chat
Choose multiple people i.e Add to Group
Click on Next
Click on the avatar or camera icon
On Web/Desktop, Verify the file explorer is open immediately
On Mobile Web, Verify it's open the options of the image source or the image explorer
On Native, Verify it's open the modal with options of image source

Also, requested the payment in ND.

JmillsExpensify · 2024-07-30T18:48:14Z

$250 approved for @mollfpr

b4s36t4 · 2024-07-31T07:13:05Z

@Christinadobrzyn here's my profile - https://www.upwork.com/freelancers/~018f450a49ceab36aa. I have also accepted the offer sent by melvin when I got assigned to the issue.

Christinadobrzyn · 2024-07-31T23:51:21Z

Regression test - https://github.com/Expensify/Expensify/issues/416898

Paid out based on this payment summary - #42868 (comment)

Closing!

jamesdeanexpensify added the External Added to denote the issue can be worked on by a contributor label May 30, 2024

melvin-bot bot changed the title ~~Remove "Upload photo" step for users who are adding a group image for the first time~~ [$250] Remove "Upload photo" step for users who are adding a group image for the first time May 30, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 30, 2024

melvin-bot bot assigned mollfpr May 30, 2024

melvin-bot bot added the Daily KSv2 label May 30, 2024

melvin-bot bot added the Overdue label Jun 3, 2024

melvin-bot bot removed the Overdue label Jun 6, 2024

nkuoch assigned b4s36t4 Jun 27, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 27, 2024

b4s36t4 mentioned this issue Jul 1, 2024

remove upload photo step if using default avatar #44702

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Jul 1, 2024

Christinadobrzyn added Daily KSv2 and removed Weekly KSv2 labels Jul 30, 2024

Christinadobrzyn changed the title ~~[$250] Remove "Upload photo" step for users who are adding a group image for the first time~~ [Payment due date 7/30][$250] Remove "Upload photo" step for users who are adding a group image for the first time Jul 30, 2024

Christinadobrzyn closed this as completed Jul 31, 2024

[Payment due date 7/30][$250] Remove "Upload photo" step for users who are adding a group image for the first time #42868

[Payment due date 7/30][$250] Remove "Upload photo" step for users who are adding a group image for the first time #42868

Comments

jamesdeanexpensify commented May 30, 2024 • edited by Christinadobrzyn Loading

melvin-bot bot commented May 30, 2024

melvin-bot bot commented May 30, 2024

abzokhattab commented May 30, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

POC

optional

Krishna2323 commented May 30, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Krishna2323 commented May 30, 2024 • edited Loading

Proposal Updated

abzokhattab commented May 30, 2024

Proposal updated

b4s36t4 commented May 30, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Jun 3, 2024

tienifr commented Jun 4, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Jun 4, 2024

melvin-bot bot commented Jun 5, 2024

mollfpr commented Jun 6, 2024

melvin-bot bot commented Jun 6, 2024

mollfpr commented Jun 7, 2024

b4s36t4 commented Jun 7, 2024

mollfpr commented Jun 7, 2024

b4s36t4 commented Jun 7, 2024

mollfpr commented Jun 7, 2024

b4s36t4 commented Jun 7, 2024

tienifr commented Jun 7, 2024

b4s36t4 commented Jun 7, 2024

mollfpr commented Jun 7, 2024

b4s36t4 commented Jun 7, 2024

Christinadobrzyn commented Jun 13, 2024

tienifr commented Jun 13, 2024

mollfpr commented Jun 13, 2024

mollfpr commented Jun 27, 2024

nkuoch commented Jun 27, 2024

melvin-bot bot commented Jun 27, 2024

Christinadobrzyn commented Jul 2, 2024

Christinadobrzyn commented Jul 9, 2024

Christinadobrzyn commented Jul 16, 2024

mollfpr commented Jul 17, 2024

melvin-bot bot commented Jul 20, 2024

melvin-bot bot commented Jul 20, 2024

Christinadobrzyn commented Jul 23, 2024

b4s36t4 commented Jul 24, 2024

b4s36t4 commented Jul 25, 2024

Christinadobrzyn commented Jul 30, 2024 • edited Loading

mollfpr commented Jul 30, 2024 • edited Loading

JmillsExpensify commented Jul 30, 2024

b4s36t4 commented Jul 31, 2024

Christinadobrzyn commented Jul 31, 2024

jamesdeanexpensify commented May 30, 2024 •

edited by Christinadobrzyn

Loading

abzokhattab commented May 30, 2024 •

edited

Loading

Krishna2323 commented May 30, 2024 •

edited

Loading

Krishna2323 commented May 30, 2024 •

edited

Loading

b4s36t4 commented May 30, 2024 •

edited

Loading

tienifr commented Jun 4, 2024 •

edited

Loading

Christinadobrzyn commented Jul 30, 2024 •

edited

Loading

mollfpr commented Jul 30, 2024 •

edited

Loading