Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix reportDetails icons for chatrooms #8011

Merged
merged 3 commits into from
Mar 4, 2022
Merged

Fix reportDetails icons for chatrooms #8011

merged 3 commits into from
Mar 4, 2022

Conversation

TomatoToaster
Copy link
Contributor

@TomatoToaster TomatoToaster commented Mar 4, 2022 •
edited
Loading

@danieldoglas, please review when you have the chance.
CC: @marcochavezf @marcaaron @roryabraham

Details

Fixes regression found here: #7852 (comment)

Fixed Issues

related to PR: #7852

Tests / QA

  1. Open a chatRoom (can be a #announce, #admin, or any policy room) and click on the header to open the report details
  2. Verify the avatar looks like this:

image

instead of this:
image

  • Verify that no errors appear in the JS console

@TomatoToaster TomatoToaster self-assigned this Mar 4, 2022
@TomatoToaster TomatoToaster changed the title Amal fix reportdetails icons Fix reportDetails icons for chatrooms Mar 4, 2022
@TomatoToaster TomatoToaster marked this pull request as ready for review March 4, 2022 16:33
@TomatoToaster TomatoToaster requested a review from a team as a code owner March 4, 2022 16:33
@MelvinBot MelvinBot requested review from danieldoglas and removed request for a team March 4, 2022 16:33
@TomatoToaster
Copy link
Contributor Author

Applying CP staging label since this is a regression and is broken on staging currently.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2022

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@danieldoglas
Copy link
Contributor

image

LGTM

danieldoglas
danieldoglas approved these changes Mar 4, 2022
View reviewed changes
Copy link
Contributor

@danieldoglas danieldoglas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danieldoglas danieldoglas merged commit d64f8c5 into main Mar 4, 2022
@danieldoglas danieldoglas deleted the amal-fix-reportdetails-icons branch March 4, 2022 16:50
OSBotify pushed a commit that referenced this pull request Mar 4, 2022
@danieldoglas @OSBotify
Merge pull request #8011 from Expensify/amal-fix-reportdetails-icons
8aa5028 
Fix reportDetails icons for chatrooms

(cherry picked from commit d64f8c5)
This was referenced Mar 4, 2022
Merged
Closed
TomatoToaster pushed a commit that referenced this pull request Mar 7, 2022
Merge pull request #8013 from Expensify/OSBotify-cherry-pick-staging-…
b0637b3 
…8011

🍒 Cherry pick PR #8011 to staging 🍒
@OSBotify
Copy link
Contributor

OSBotify commented Mar 7, 2022

🚀 Cherry-picked to staging by @danieldoglas in version: 1.1.41-5 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@kbecciv kbecciv mentioned this pull request Mar 8, 2022
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to production by @francoisl in version: 1.1.41-6 🚀

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

@OSBotify OSBotify mentioned this pull request Mar 11, 2022
Closed
@SumitDiyora SumitDiyora mentioned this pull request Apr 20, 2022
Closed
22 tasks
@OSBotify OSBotify mentioned this pull request Aug 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danieldoglas danieldoglas danieldoglas approved these changes

Assignees

@TomatoToaster TomatoToaster

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TomatoToaster @danieldoglas @OSBotify