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

Add ability to dismiss attachment modal via swipe #2442

Merged
merged 11 commits into from
Apr 19, 2021

Conversation

Jag96
Copy link
Contributor

@Jag96 Jag96 commented Apr 16, 2021

@marcaaron will you please review this?

Details

This PR adds the ability to swipe down and right to dismiss an attachment on mobile.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/154788

Tests

  1. Sign into an account and send an image attachment
  2. After the attachment uploads, tap on it, confirm an attachment modal shows
  3. Double tap on the image, confirm it zooms
  4. Double-tap again, confirm it zooms out
  5. Use two fingers to zoom in, confirm the image zooms in
  6. While zoomed in, swipe down and right and confirm the modal doesn't close, and that the viewport pans around the zoomed image
  7. Double-tap again then swipe down or right, confirm the modal dismisses
  8. Confirm that there are no regressions on web
    1. Confirm attachments still show up on web
    2. Confirm the modal attachment closes normally on web

QA Steps

Same as above

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

@Jag96 Jag96 requested a review from marcaaron April 16, 2021 20:45
@Jag96 Jag96 requested a review from a team as a code owner April 16, 2021 20:45
@Jag96 Jag96 self-assigned this Apr 16, 2021
@MelvinBot MelvinBot requested review from nkuoch and removed request for a team April 16, 2021 20:45
@Jag96 Jag96 changed the title Joe dismiss image preview swipe Add ability to dismiss attachment modal via swipe Apr 16, 2021
@@ -402,6 +402,8 @@ PODS:
- React-Core
- RNCAsyncStorage (1.12.1):
- React-Core
- RNCClipboard (1.5.1):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this wasn't updated previously, doing a pod install added these.

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.

LGTM!

@marcaaron marcaaron merged commit 0f83e24 into main Apr 19, 2021
@marcaaron marcaaron deleted the joe-dismiss-image-preview-swipe branch April 19, 2021 16:38
@OSBotify
Copy link
Contributor

🚀 [Deployed](https://github.com/Expensify/Expensify.cash
/actions/runs/765130740) 🚀 to
staging on Mon Apr 19 2021 at 22:41:56 GMT+0000 (Coordinated Universal Time)

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

@isagoico
Copy link

Android/iOS - Chat - Image does not zoom in when two fingers are used

Expected Result

Confirm the image zooms in

Actual Result

Image does not zoom in

Action Performed

  1. Launch the app and login
  2. Send an image attachment
  3. After the attachment uploads, tap on it, confirm an attachment modal shows
  4. Use two fingers to zoom in

Platform

iOS ✔️
Android ✔️

Notes/Images/Videos
Issue is reproducible in production but it's failing step 5 of the PR. Let me know if I should raise this as a separate issue.

Bug5030887_20210420_162938_1_.mp4

@Jag96
Copy link
Contributor Author

Jag96 commented Apr 20, 2021

@isagoico if this is happening on production then let's create a new issue for it, feel free to tag me in it and I'll have a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants