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

[CP Stg] Resolve getImageSize promise with an object instead of multiple values #7414

Merged
merged 1 commit into from
Jan 26, 2022
Merged

[CP Stg] Resolve getImageSize promise with an object instead of multiple values #7414

merged 1 commit into from
Jan 26, 2022

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Jan 26, 2022 •
edited
Loading

Details

Those pesky curly braces! The problem here was that Promise.resolve only takes a single argument, not multiple args. So we now pass width and height together in a single object.

Fixed Issues

$ #7407

Tests / QA Steps

  1. Send an image attachment that's not square.
  2. Make sure it doesn't appear inscribed within the square thumbnail.
  • Verify that no new errors appear in the JS console (tests)
  • Verify that no new errors appear in the JS console (QA)

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

Desktop

image

iOS

Android

@roryabraham roryabraham requested a review from marcaaron January 26, 2022 00:36
@roryabraham roryabraham self-assigned this Jan 26, 2022
@roryabraham roryabraham requested a review from a team as a code owner January 26, 2022 00:36
@github-actions
Copy link
Contributor

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

@MelvinBot MelvinBot requested review from joelbettner and removed request for a team January 26, 2022 00:37
@roryabraham
Copy link
Contributor Author

All screenshots are up and this is ready for review!

marcaaron
marcaaron approved these changes Jan 26, 2022
View reviewed changes
Copy link
Contributor

@marcaaron marcaaron left a comment

Choose a reason for hiding this comment

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

Oh woops nice catch @roryabraham 😅

@marcaaron marcaaron merged commit 87eda2a into main Jan 26, 2022
@marcaaron marcaaron deleted the Rory-FixImageResizing branch January 26, 2022 01:28
OSBotify pushed a commit that referenced this pull request Jan 26, 2022
@marcaaron @OSBotify
Merge pull request #7414 from Expensify/Rory-FixImageResizing
97f201c 
[CP Stg] Resolve getImageSize promise with an object instead of multiple values

(cherry picked from commit 87eda2a)
This was referenced Jan 26, 2022
Merged
Closed
@OSBotify
Copy link
Contributor

🚀 Cherry-picked to staging by @marcaaron in version: 1.1.33-2 🚀

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.

@OSBotify
Copy link
Contributor

OSBotify commented Feb 1, 2022

🚀 Deployed to production by @roryabraham in version: 1.1.33-3 🚀

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

@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

@marcaaron marcaaron marcaaron approved these changes

@joelbettner joelbettner Awaiting requested review from joelbettner

Assignees

@roryabraham roryabraham

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@roryabraham @OSBotify @marcaaron