Add SVG Support and Update Default Avatars for New Dot 2023

[grgia]

Details

This PR adds our new default avatars as Local SVGs following the pattern of our workspace avatars.

• Adds support everywhere for Avatar components (Avatar, MultipleAvatars, RoomHeaderAvatars, SubscriptAvatar, AvatarWithIndicator, AvatarWithImagePicker) to allow SVG functions as source
• Adds SVG preview support to Attachment flows
• Adds Local Concierge SVG Avatar
• Note: The user can still not upload an SVG as an avatar because SVGs must be supported on both old/new dot

Previously, the avatar URL from CloudFront was stored in the user's NVPs. In addition, we would hash the users email to get the default avatar source URL from CloudFront.

We will use this same general method, but now, we first check if the account requires a default avatar and if so, we get the local SVG for the user's default avatar and use that instead.

General flows:

Screen.Recording.2023-01-04.at.7.18.55.PM.mov

Old dot & New Dot upload/delete (note: old dot does not automatically render):

Screen.Recording.2023-01-04.at.7.38.26.PM.mov

New account flow:

Screen.Recording.2023-01-05.at.12.42.03.PM.mov

Fixed Issues

#12259 (1/2 - User Default Avatars)

Tests

• Verify that no errors appear in the JS console

Offline tests

• Verify that no errors appear in the JS console

• Verify New User Flow Avatars Match

  • On the sign-up page, sign up with a new email. Note the avatar shown.
  • Verify the account and log in, and ensure that the avatar noted matches the assigned avatar.
  • Repeat steps with SMS #

• Verify Avatar can be changed to a custom photo, and can be deleted

  • Change the avatar to a custom photo and verify that it is changed on both old dot and new dot.
  • Delete the avatar, verify the account has an old default avatar on old dot and a new avatar on new dot

• Verify Concierge Icon is correct for both old dot and new dot

• Verify Avatar Previews work

  • For Default Avatar
  • For Concierge Avatar (note, circular)
  • For User-Uploaded Avatar

• Verify Chat Avatar matches in all places

  • Profile
  • Chat
  • About User
  • Searching for User

• Create an empty chat with a new user, verify that a new avatar is used.

• Verify Attachment flows still work

  • Try uploading an SVG - Verify it DOES NOT allow you
  • Upload an image, Verify attachment uploads, and preview works properly
  • Upload a GIF, Verify attachment uploads, and preview works properly
  • Upload a PDF, Verify attachment uploads, and preview works properly

• Go offline and Upload a new user avatar

  • Verify that every place your avatar shows, now shows a grayed-out version of the new avatar (pending UI)
  • Go back online and Verify every place your avatar shows, now shows the full (not grayed out) version of the avatar

• Verify Chat Avatar Works in Workspace Invite Flow

  • Go to a Workspace > Invite members
  • Verify that the list of avatars shows any custom avatars and correct default avatars
  • In the Search, type an account that doesn't exit. Verify that the avatar shown is correct.
  • Add Users to your workspace. Go back to LHN. Verify that the subscript avatar looks correct.
  • Click on a user icon that has a workspace subscript. Verify chat header/body icon is correct.

-[x] Verify that #focus mode looks correct

• Go to settings > preferences > turn on focus mode
• Ensure that group chat avatars are 18x18px
• Ensure all other kinds of avatars in compact mode are 28x28px

QA Steps

• Verify that no errors appear in the JS console

• Verify New User Flow Avatars Match

  • On the sign-up page, sign up with a new email. Note the avatar shown.
  • Verify the account and log in, and ensure that the avatar noted matches the assigned avatar.
  • Repeat steps with SMS #

• Verify Avatar can be changed to a custom photo, and can be deleted

  • Change the avatar to a custom photo and verify that it is changed on both old dot and new dot.
  • Delete the avatar, verify the account has an old default avatar on old dot and a new avatar on new dot

• Verify Concierge Icon is correct for both old dot and new dot

• Verify Chat Avatar matches in all places

  • Profile
  • Chat
  • About User
  • Searching for User

• Verify Avatar Previews work

  • For Default Avatar
  • For Concierge Avatar (note, circular)
  • For User-Uploaded Avatar

• Create an empty chat with a new user, verify that a new avatar is used.

• Verify Attachment flows still work

  • Try uploading an SVG - Verify it DOES NOT allow you
  • Upload an image, Verify attachment uploads, and preview works properly
  • Upload a GIF, Verify attachment uploads, and preview works properly
  • Upload a PDF, Verify attachment uploads, and preview works properly

• Go offline and Upload a new user avatar

  • Verify that every place your avatar shows, now shows a grayed-out version of the new avatar (pending UI)
  • Go back online and Verify every place your avatar shows, now shows the full (not grayed out) version of the avatar

• Verify Chat Avatar Works in Workspace Invite Flow

  • Go to a Workspace > Invite members
  • Verify that the list of avatars shows any custom avatars and correct default avatars
  • In the Search, type an account that doesn't exit. Verify that the avatar shown is correct.
  • Add Users to your workspace. Go back to LHN. Verify that the subscript avatar looks correct.
  • Click on a user icon that has a workspace subscript. Verify chat header/body icon is correct.

-[ ] Verify that #focus mode looks correct

• Go to settings > preferences > turn on focus mode
• Ensure that group chat avatars are 18x18px
• Ensure all other kinds of avatars in compact mode are 28x28px

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • iOS / native
  • Android / native
  • iOS / Safari
  • Android / Chrome
  • MacOS / Chrome
  • MacOS / Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

See description for videos.

Mobile Web - Chrome

Screen.Recording.2023-01-04.at.7.53.47.PM.mov

Mobile Web - Safari

Screen.Recording.2023-01-04.at.7.56.53.PM.mov

Desktop

iOS

Screen.Recording.2023-01-04.at.7.55.21.PM.mov

Android

Screen.Recording.2023-01-04.at.7.52.51.PM.mov

[grgia]

Update: holding while we finalize core branding fixes

[grgia]

Planning to update these by early next week!

[grgia]

Pushed an update, I still want to clean up the code and try and think of any edge cases that might break things, but so far working well with SVGs!

[grgia]

Hey @shawnborton I just want to double-check on this sizing for avatar previews (300x300)

Mobile Safari	IOS iphone 14	Mobile Chrome	Android	Desktop	Web

[shawnborton]

I think the preview size feels pretty good. I suppose we could bump it up to 400px so that it feels "full width" on mobile at least? What do you think?

[grgia]

Right now I have it at 360:

Here's a preview of 400:

[grgia]

Updated again, thank you @mananjadhav !

[mananjadhav]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• I verified testing steps are clear and they cover the changes made in this PR
  • I verified the steps for local testing are in the Tests section
  • I verified the steps for Staging and/or Production testing are in the QA steps section
  • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
• I checked that screenshots or videos are included for tests on all platforms
• I included screenshots or videos for tests on all platforms
• I verified tests pass on all platforms & I tested again on:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I verified that this PR follows the guidelines as stated in the Review Guidelines
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

web-default-avatar_ZXyZrmJl.mp4

Mobile Web - Chrome

mweb-chrome-default-avatar.mov

Mobile Web - Safari

mweb-safari-default-avatar.mov

Desktop

desktop-default-avatar.mov

iOS

ios-default-avatar.mov

Android

android-default-avatar.mov

[mananjadhav]

@grgia I've tried my best to cover all the places where we've got Avatars. This tests well but I just saw we've got Merge conflict. I can see it's only in one file OptionRow.js so once you're done I'll do a quick test and let's merge this asap. I've completed the checklist anyway :)

Thank you so much for catering to all the feedback and we're almost at the finish line.

@Beamanator @arosiclair if you folks can review this and we can merge today we won't end up resolving more conflicts later.

[Beamanator]

Looking great, I would say it's ready for merge, except for the tiny merge conflict 😬

[grgia]

Fixed merged conflict! @mananjadhav

[Beamanator]

Merging so we don't get any more conflicts :D

[arosiclair]

LGTM

[OSBotify]

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

[github-actions]

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	701.387 ms → 734.539 ms (+33.152 ms, +4.7%)
App start runJsBundle	196.548 ms → 201.531 ms (+4.983 ms, +2.5%)
App start nativeLaunch	19.552 ms → 20.125 ms (+0.573 ms, +2.9%)
App start regularAppStart	0.015 ms → 0.016 ms (+0.001 ms, +7.1%)
Open Search Page TTI	612.214 ms → 607.694 ms (-4.520 ms, -0.7%)

Show details

Name	Duration
App start TTI	Baseline Mean: 701.387 ms Stdev: 22.130 ms (3.2%) Runs: 658.0311459992081 661.4998359996825 671.0722300000489 680.3327050004154 682.7979579996318 685.4757230002433 685.6311859991401 686.4672340005636 687.6567369997501 687.8426210004836 690.3496449999511 694.433382999152 696.2014130000025 701.3613479994237 702.5273030009121 702.6073749996722 704.0666380003095 705.1415059994906 706.074822999537 707.3352540004998 708.4181280005723 708.5400430001318 711.5800490006804 711.7549809999764 711.9457629993558 718.3977899998426 730.8191390000284 739.863628000021 748.7758290003985 754.6187880001962 Current Mean: 734.539 ms Stdev: 23.143 ms (3.2%) Runs: 691.9181730002165 697.9511130005121 699.3977799993008 704.8619660008699 706.547983000055 711.0419960003346 713.0131449997425 716.1406239997596 717.1622199993581 718.6748530007899 720.8774609994143 723.7904770001769 724.702235000208 725.2760930005461 726.5955049991608 733.4674079995602 734.3409460000694 741.7213279996067 742.3290410004556 742.9076579995453 746.9056369997561 748.2596150003374 748.7624180000275 750.2650579996407 752.5024980008602 757.7377780005336 762.5081740003079 767.0406180005521 767.1087420005351 768.073153000325 769.2712099999189 774.0906290002167
App start runJsBundle	Baseline Mean: 196.548 ms Stdev: 17.142 ms (8.7%) Runs: 166 171 173 174 175 181 182 185 187 188 188 189 191 192 194 195 195 196 196 198 201 204 209 213 217 218 218 218 222 223 234 Current Mean: 201.531 ms Stdev: 19.120 ms (9.5%) Runs: 176 176 177 178 183 184 184 185 186 188 188 188 191 194 194 196 202 202 202 203 203 205 215 215 216 224 225 228 229 234 239 239
App start nativeLaunch	Baseline Mean: 19.552 ms Stdev: 1.379 ms (7.1%) Runs: 18 18 18 18 18 18 18 18 19 19 19 19 19 19 19 19 20 20 20 20 20 20 21 21 21 21 22 22 23 Current Mean: 20.125 ms Stdev: 2.522 ms (12.5%) Runs: 17 18 18 18 18 18 18 18 18 18 18 19 19 19 19 19 19 19 19 20 21 21 21 21 21 22 23 24 24 25 26 26
App start regularAppStart	Baseline Mean: 0.015 ms Stdev: 0.001 ms (7.2%) Runs: 0.012736000120639801 0.013020999729633331 0.01355000026524067 0.013955999165773392 0.013956999406218529 0.013996999710798264 0.01407800056040287 0.014241000637412071 0.014281999319791794 0.014363998547196388 0.014403998851776123 0.014403998851776123 0.014485999941825867 0.014607999473810196 0.014649000018835068 0.014649000018835068 0.014649000018835068 0.014730000868439674 0.014852000400424004 0.014973999932408333 0.015136001631617546 0.015543000772595406 0.015584999695420265 0.015584999695420265 0.015991998836398125 0.016112999990582466 0.01619499921798706 0.016234999522566795 0.01643799990415573 0.016683001071214676 0.016805000603199005 0.017131000757217407 Current Mean: 0.016 ms Stdev: 0.001 ms (4.9%) Runs: 0.014607999473810196 0.014730000868439674 0.014810999855399132 0.01505499891936779 0.015096001327037811 0.015135999768972397 0.015137000009417534 0.015381000936031342 0.01550300046801567 0.015584001317620277 0.015625 0.0157880000770092 0.015869000926613808 0.015992000699043274 0.016071999445557594 0.016112999990582466 0.016114000231027603 0.016154000535607338 0.01619499921798706 0.01619500108063221 0.016234999522566795 0.01635799929499626 0.016479000449180603 0.016519999131560326 0.016560999676585197 0.016641998663544655 0.016683001071214676 0.016885999590158463 0.017455998808145523 0.017496999353170395 0.017619000747799873
Open Search Page TTI	Baseline Mean: 612.214 ms Stdev: 20.043 ms (3.3%) Runs: 578.206664999947 585.222372001037 586.0430509988219 588.7001949995756 589.5701899994165 594.5169679988176 595.5204670000821 596.1624759994447 596.3199460003525 597.637085000053 597.9859219994396 600.0379230007529 602.2429609987885 603.8538000006229 604.8678389992565 604.8796389997005 605.3005790002644 606.6579999998212 619.0869959983975 620.3548999987543 621.0418290011585 622.6401780005544 629.2669679988176 629.8546959999949 631.5185550004244 631.9163409993052 632.7539469990879 634.4886480011046 643.8516850005835 644.3719890005887 644.6944180000573 651.288575001061 Current Mean: 607.694 ms Stdev: 21.008 ms (3.5%) Runs: 574.3582359999418 580.4518229998648 581.7431639991701 582.9379879999906 583.3472499996424 583.7196050006896 584.9545900002122 585.2757570017129 593.7268890012056 596.1481120008975 596.5551770012826 598.2407229989767 598.9929200001061 599.0820310004056 601.1300869993865 604.9731449987739 605.1652019992471 607.8520100004971 608.1017660014331 608.6786299999803 609.5268149990588 609.6459560003132 616.3148200009018 618.6278489995748 619.1496989987791 623.0745860002935 623.8186039999127 636.7533780001104 638.8918049987406 639.4525959994644 645.5810959991068 646.0649420004338 651.5638030003756

[mananjadhav]

Thanks for resolving the conflicts @grgia. I just did a basic test on main branch and it works well. Glad we merged before another conflict.

[OSBotify]

🚀 Deployed to staging by https://github.com/Beamanator in version: 1.2.64-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[parasharrajat]

On Workspace Invite Member Page, images are not updated immediately for newly invited users while the user is offline. This happens because the workspace member invite page was not updated to remove the old Fallback image.

Here is the regression which was missed during this refactor #14819