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 isComponentMounted usages #7327

Merged
merged 4 commits into from
Jan 20, 2022
Merged

Fix isComponentMounted usages #7327

merged 4 commits into from
Jan 20, 2022

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Jan 19, 2022 •
edited
Loading

cc @nickmurray47

Details

  • Gets rid of a couple of usages of isComponentMounted and replaces them with makeCancellablePromise()
  • Also stops throwing inside makeCancellablePromise() as cancelling should be allowed and not throw an error.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/179650

Tests / QA Steps

  1. Test tooltip behavior and verify it still works
  2. Upload image attachment and view attachments in modals verify there are no issues.
  • Verify that no errors appear in the JS console

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@marcaaron marcaaron self-assigned this Jan 19, 2022
@marcaaron marcaaron marked this pull request as ready for review January 19, 2022 19:35
@marcaaron marcaaron requested a review from a team as a code owner January 19, 2022 19:35
@MelvinBot MelvinBot requested review from TomatoToaster and removed request for a team January 19, 2022 19:35
nickmurray47
nickmurray47 approved these changes Jan 19, 2022
View reviewed changes
Copy link
Contributor

@nickmurray47 nickmurray47 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tests well and LGTM, thanks for the update!

@TomatoToaster TomatoToaster merged commit d9c483c into main Jan 20, 2022
@TomatoToaster TomatoToaster deleted the marcaaron-isComponentMounted branch January 20, 2022 21:04
@OSBotify
Copy link
Contributor

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

@OSBotify OSBotify mentioned this pull request Jan 20, 2022
Closed
33 tasks
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @TomatoToaster in version: 1.1.31-2 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @AndrewGable in version: 1.1.32-0 🚀

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

@roryabraham roryabraham mentioned this pull request Jan 26, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickmurray47 nickmurray47 nickmurray47 approved these changes

@TomatoToaster TomatoToaster TomatoToaster approved these changes

Assignees

@marcaaron marcaaron

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marcaaron @OSBotify @TomatoToaster @nickmurray47