[User Settings] Allow users to modify avatar photo #2131

Maftalion · 2021-03-28T23:05:44Z

Details

Refactored createmenu to accept menu items as props
Added upload/delete avatar functionality
Delete avatar will only populate if avatar is not a default avatar
[existing bug] Popover on web/desktop appears in bottom left corner. CreateMenu should display within the chat window area #2090

Fixed Issues

Fixes #2053

Tests

Go to Settings -> Profile by clicking your avatar
Click Edit Photo, and upload a new photo
If you remove or have a default photo, the remove photo option should not appear

QA Steps

same as above

Tested On

Video Walkthrough

Screen.Recording.2021-03-28.at.3.52.42.PM.mov

Screenshots

Web

Mobile Web

Desktop

iOS

Android

src/pages/settings/ProfilePage.js

shawnborton · 2021-03-29T08:12:48Z

Here is the upload icon you need: upload.svg.zip

deetergp

The code looks good to me, but I cannot seem to get the actual upload functionality to work. When I attempt to upload, I end up seeing the following in my logs:

2021-03-29T21:55:53.273159+00:00 expensidev php-fpm: cLiamR /api.php scott@expensify.com !https://expensify.cash/! ?user? [info] [push] Push_Service - Push_Service sending event ~~ channel: '[0: 'public-debugserverid5e068a4783949']' eventName: 'debug' eventData: '{"level":"warn","message":"UserAPI - Empty string provided as image when uploading avatar."}'

I'm seeing this same behavior on both the platforms I tested: Web & iOS.

NikkiWines · 2021-03-29T22:05:42Z

@deetergp are you to date on master on web? We merged a Web-Expensify change for this functionality. Just tested myself on web and it's working well 👍 gonna leave a full review as well.

deetergp · 2021-03-29T22:31:18Z

@NikkiWines That was it, @NikkiWines, thanks! Was able to verify that it works on Web, iOS, & Desktop, but could not get it to work on Mobile Web, where it failed to open a picker:
https://user-images.githubusercontent.com/766197/112908048-a78c7080-90a3-11eb-8884-feefbdc4118a.mp4

NikkiWines

Some small comments, this is looking great! I find CreateMenu much more intuitive now that we're not maintaining that list inside of it.

One thing on the design, I personally quite like having the create menu follow the style we have currently for web/desktop (i.e. sliding out from the bottom left) but @michelle-thompson do you want it to pop up below the Edit Photo button like we have it in the mockups?

src/libs/actions/PersonalDetails.js

src/pages/home/report/ReportActionCompose.js

src/components/CreateMenu.js

src/libs/actions/PersonalDetails.js

NikkiWines · 2021-03-29T23:23:02Z

Also was able to reproduce the issue @deetergp experienced with mWeb not opening a picker.

michelle-thompson · 2021-03-29T23:35:51Z

I see your point about all other menu appearing in the bottom left, but let's position the menu underneath Edit Photo so it's clear what the menu actions would be affecting.

Maftalion · 2021-03-29T23:40:42Z

@NikkiWines @michelle-thompson the popover location seemed to be an existing bug - #2090

I was waiting on that issue to be solved to see how to set it up here as well

Maftalion · 2021-03-29T23:54:32Z

mWeb file attachment seems to be broken on current master as well

shawnborton · 2021-03-30T07:59:24Z

I agree with @michelle-thompson, we should launch this menu in relation to the button that is tapped. In terms of animating the popover, I had a comment in a different PR that Rory addressed in terms of making the popover appear to animate downwards from the button location. Maybe this is helpful: #1878 (comment)

NikkiWines · 2021-03-31T01:46:04Z

Since both the attachment picker mWeb issue and the popover issue exist on production at the moment I don't think we should block this PR on those being resolved (especially since I don't think either issue has a PR out yet).

I can make a follow-up issue to move the popover location to underneath the Edit Photo button if we agree that moving ahead for now is the right call.

NikkiWines

One minor comment, otherwise looking great

src/libs/actions/PersonalDetails.js

NikkiWines

This is looking great and code looks good to me. A comment on one thing I've noticed.

Uploading a photo - or removing a photo - updates some of the avatars (profile page, sidebar) but doesn't immediately update the avatars in the chats.

On mobile: It's pretty delayed, even when navigated to-and-away from reports. Not sure how much of this is due to it being tested on a sim.

Screen.Recording.2021-03-31.at.12.05.40.PM.mov
On web/desktop: You can get the photo to update by navigating away and back to a report

Screen.Recording.2021-03-31.at.11.36.50.AM.mov

NikkiWines

Ok looks like the delayed avatar issue is related to an existing issue: https://github.com/Expensify/Expensify/issues/155256 - so let's not hold on that.

I think we should merge this and I'll create a separate issue to move the CreateMenu location to underneath the Edit Photo button once the underlying issue for that has been resolved.

How does that sound to you all?

deetergp · 2021-04-01T23:49:57Z

src/components/CreateMenu.js

-        PropTypes.oneOf(Object.values(CONST.MENU_ITEM_KEYS)),
+    menuItems: PropTypes.arrayOf(
+        PropTypes.shape({
+            icon: PropTypes.func.isRequired,


NAB: Just curious why this prop type is func. Isn't it a PNG?

deetergp

Looks good to me 👍

OSBotify · 2021-04-01T23:51:02Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

mallenexpensify · 2021-04-07T16:43:42Z

Got here from https://expensify.slack.com/archives/C01GTK53T8Q/p1617743567403300
Curious if we need a new issue for "Add Attachment" or if it will be addressed here

Maftalion · 2021-04-07T16:45:47Z

@mallenexpensify Opened a pr here to fix that - #2254

Maftalion added 4 commits March 26, 2021 17:20

initial avatar setup

64f7d36

refactor createmenu, add upload/delete avatar

6e591a3

lint

41ffe3a

Merge branch 'master' into matt-2053-edit-avatar

f8d87a7

Maftalion requested a review from a team as a code owner March 28, 2021 23:05

botify requested review from deetergp and removed request for a team March 28, 2021 23:06

Maftalion commented Mar 29, 2021

View reviewed changes

src/pages/settings/ProfilePage.js Outdated Show resolved Hide resolved

Add Upload icon

69fa80f

deetergp reviewed Mar 29, 2021

View reviewed changes

NikkiWines requested changes Mar 29, 2021

View reviewed changes

src/libs/actions/PersonalDetails.js Outdated Show resolved Hide resolved

src/pages/home/report/ReportActionCompose.js Outdated Show resolved Hide resolved

src/components/CreateMenu.js Outdated Show resolved Hide resolved

src/libs/actions/PersonalDetails.js Outdated Show resolved Hide resolved

Merge branch 'master' into matt-2053-edit-avatar

52ca160

pr feedback fixes

ffd016e

NikkiWines requested changes Mar 31, 2021

View reviewed changes

src/libs/actions/PersonalDetails.js Outdated Show resolved Hide resolved

update comment

14c0fae

NikkiWines reviewed Mar 31, 2021

View reviewed changes

NikkiWines approved these changes Apr 1, 2021

View reviewed changes

deetergp reviewed Apr 1, 2021

View reviewed changes

deetergp approved these changes Apr 1, 2021

View reviewed changes

deetergp merged commit 5794592 into Expensify:master Apr 1, 2021

Maftalion mentioned this pull request Apr 7, 2021

Fix "Add Attachment" Menu Item #2254

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[User Settings] Allow users to modify avatar photo #2131

[User Settings] Allow users to modify avatar photo #2131

Maftalion commented Mar 28, 2021

shawnborton commented Mar 29, 2021

deetergp left a comment

NikkiWines commented Mar 29, 2021 •

edited

Loading

deetergp commented Mar 29, 2021

NikkiWines left a comment •

edited

Loading

NikkiWines commented Mar 29, 2021

michelle-thompson commented Mar 29, 2021

Maftalion commented Mar 29, 2021

Maftalion commented Mar 29, 2021

shawnborton commented Mar 30, 2021

NikkiWines commented Mar 31, 2021

NikkiWines left a comment

NikkiWines left a comment •

edited

Loading

NikkiWines left a comment

deetergp Apr 1, 2021

deetergp left a comment

OSBotify commented Apr 1, 2021

mallenexpensify commented Apr 7, 2021

Maftalion commented Apr 7, 2021

[User Settings] Allow users to modify avatar photo #2131

[User Settings] Allow users to modify avatar photo #2131

Conversation

Maftalion commented Mar 28, 2021

Details

Fixed Issues

Tests

QA Steps

Tested On

Video Walkthrough

Screenshots

Web

Mobile Web

Desktop

iOS

Android

shawnborton commented Mar 29, 2021

deetergp left a comment

Choose a reason for hiding this comment

NikkiWines commented Mar 29, 2021 • edited Loading

deetergp commented Mar 29, 2021

NikkiWines left a comment • edited Loading

Choose a reason for hiding this comment

NikkiWines commented Mar 29, 2021

michelle-thompson commented Mar 29, 2021

Maftalion commented Mar 29, 2021

Maftalion commented Mar 29, 2021

shawnborton commented Mar 30, 2021

NikkiWines commented Mar 31, 2021

NikkiWines left a comment

Choose a reason for hiding this comment

NikkiWines left a comment • edited Loading

Choose a reason for hiding this comment

NikkiWines left a comment

Choose a reason for hiding this comment

deetergp Apr 1, 2021

Choose a reason for hiding this comment

deetergp left a comment

Choose a reason for hiding this comment

OSBotify commented Apr 1, 2021

mallenexpensify commented Apr 7, 2021

Maftalion commented Apr 7, 2021

NikkiWines commented Mar 29, 2021 •

edited

Loading

NikkiWines left a comment •

edited

Loading

NikkiWines left a comment •

edited

Loading