update Avatars#8187
Conversation
|
In the OP the expected result was to change the icons for #announce and #admins room but the zip file that @shawnborton had provided also contains a SVG for private room. So, do we wanna show a different Avatar for private rooms? If yes then should I add a function which checks if the room is both user-made and private? |
|
Let's not worry about the private room functionality for now, we can follow up with that in a separate project. |
rushatgabhane
left a comment
There was a problem hiding this comment.
@thesahindia to be pedantic
…date-icon merge main
|
I have made the suggested changes, it's ready for the review now. |
There was a problem hiding this comment.
Thanks for the changes @thesahindia
@stitesExpensify LGTM! 🎉
PR Reviewer Checklist
- 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 verified tests pass on all platforms & I tested again on:
- iOS / native
- Android / native
- iOS / Safari
- Android / Chrome
- MacOS / Chrome
- MacOS / Desktop
- I verified there are no console errors related to changes in this PR
- 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 was added in all src/languages/* files
- I verified any copy / text that was added to the app is correct english and approved by marketing by tagging the marketing team 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
- I verified that this PR follows the guidelines as stated in the Review Guidelines
- I verified other components are not impacted by changes in this PR (i.e. if the PR modifies a shared library or component like
Avatar, I verified the components usingAvatarare working as expected) - I verified the UI performance was not affected (the performance is the same than
mainbranch) - 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 */
- Any functional components have the displayName property
- 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
- Any internal methods are bound to “this” properly so there are no scoping issues
- Any internal methods bound to “this” are necessary to be bound
- 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 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)
|
🚀 Deployed to staging by @stitesExpensify in version: 1.1.44-0 🚀
|
Details
Added different Avatar for #announce and #admins room
Changed the Avatar of rooms and deleted rooms
Fixed Issues
$ #8147
Tests | QA Steps
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectionsrc/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNameproperty(i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)PR Reviewer Checklist
### Fixed Issuessection aboveTestssectionQA stepssectionsrc/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNameproperty(i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Screenshots
Web
Screen.Recording.2022-03-17.at.1.24.21.AM.mov
Mobile Web
Screen.Recording.2022-03-17.at.1.49.29.AM.mov
Desktop
Screen.Recording.2022-03-17.at.1.27.46.AM.mov
iOS
Screen.Recording.2022-03-17.at.1.51.50.AM.mov
Android
Screen.Recording.2022-03-17.at.1.29.41.AM.mov