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.

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

fix: Document Picker crash on IOS #3581

Merged
merged 1 commit into from
Jun 14, 2021

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented Jun 14, 2021

Details

We had to put a delay between modal is closed and picker opens the document chooser.

Fixed Issues

Fixes #3543

Tests | QA Steps

  1. Open any conversation on E.cash mobile app.
  2. click Add attachment from plus icon.
  3. Select the add attachment option and

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

ios-picker.mp4

Android

1623693284878.mp4

@parasharrajat parasharrajat requested a review from a team as a code owner June 14, 2021 18:00
@MelvinBot MelvinBot requested review from ctkochan22 and removed request for a team June 14, 2021 18:01
Copy link
Contributor

@Julesssss Julesssss left a comment

Choose a reason for hiding this comment

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

I was going to suggest reducing this to 100ms, as 200ms is quite noticeable as a user. But that's how we got into this mess in the first place, so I won't block on that.

@parasharrajat
Copy link
Member Author

I checked that 100ms does not work. but I am looking into other factors which cause this setTimout. No luck so far.

@roryabraham
Copy link
Contributor

Great job finding the solution here @parasharrajat! 🎉

Do you know if there is an issue in the react-native-modal repo for this? It seems that onModalHide is being executed before the modal is completely closed, which it should not. If there's no issue, should we consider opening one?

@parasharrajat
Copy link
Member Author

parasharrajat commented Jun 14, 2021

They do seem to fix it here by calling it after state update. https://github.com/react-native-modal/react-native-modal/blob/b580105f4b2b4e1163bed1228f241b16ea17dacb/src/modal.tsx#L697. but I don't think it can be improved further but nobody knows. It could be an issue with the React-native modal. Not sure about opening another issue. there are a couple of closed ones there.

@parasharrajat
Copy link
Member Author

@roryabraham it seems we missed the Slack Webhook Url Env for Gh actions.

@roryabraham
Copy link
Contributor

roryabraham commented Jun 14, 2021

@roryabraham it seems we missed the Slack Webhook Url Env for Gh actions.

This is a known issue for GH Actions where your repo secrets randomly disappear for some workflow runs for no reason. 😠

@ctkochan22
Copy link
Contributor

Confirmed with @roryabraham that we are good to merge

@ctkochan22 ctkochan22 merged commit 5a04371 into Expensify:main Jun 14, 2021
github-actions bot pushed a commit that referenced this pull request Jun 14, 2021
fix: Document Picker crash on IOS
(cherry picked from commit 5a04371)
@OSBotify
Copy link
Contributor

🚀 Cherry-picked to staging in version: 1.0.68-4🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.68-4🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.68-5🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.73-3🚀

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