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 PDF Attachments not scrollable #3487

Merged
merged 4 commits into from
Jun 15, 2021
Merged

Fix PDF Attachments not scrollable #3487

merged 4 commits into from
Jun 15, 2021
+42 −1

Conversation

rdjuric
Copy link
Contributor

@rdjuric rdjuric commented Jun 9, 2021

Details

  1. Creates a variation of the CENTERED modal type that cannot be dismissed with a swipe, called CENTERED_UNSWIPEABLE
  2. In the AttachmentModal, uses the CENTERED_UNSWIPEABLE type when the attachment is a PDF
  3. Adds the propagateSwipe prop to our BaseModal

Fixed Issues

Fixes #3247

Tests

  1. Launch the app
  2. Log in with any account and select any user
  3. Click on add attachment
  4. Upload a PDF with minimum 2 pages
  5. Open the PDF preview in the conversation
  6. The PDF is scrollable

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web.mov

Mobile Web

mWeb.mov

Desktop

desktop.mov

iOS

ios.mov

Android

android1.mp4

Sorry, something went wrong.

rdjuric added 3 commits June 9, 2021 02:51
@rdjuric
makes PDF attachment modals scrollable
539cf49
@rdjuric
uses the correct modal type

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
91f6422
@rdjuric
correctly passes the propateSwipe prop

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
af77302
@rdjuric rdjuric requested a review from a team as a code owner June 9, 2021 18:03
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2021 •
edited
Loading

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@MelvinBot MelvinBot requested review from ctkochan22 and removed request for a team June 9, 2021 18:03
@rdjuric
fix style

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
c574127
@rdjuric rdjuric mentioned this pull request Jun 9, 2021
Closed
@rdjuric
Copy link
Contributor Author

rdjuric commented Jun 9, 2021

I have read the CLA Document and I hereby sign the CLA

@rdjuric
Copy link
Contributor Author

rdjuric commented Jun 14, 2021

Not sure if I need to do anything else for the CLA test to pass, but this is ready for review @ctkochan22

@ctkochan22 ctkochan22 merged commit a722ceb into Expensify:main Jun 15, 2021
@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 Jun 15, 2021
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.68-5🚀

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

@rdjuric rdjuric mentioned this pull request Jun 22, 2021
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.73-3🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iwiznia iwiznia

@ctkochan22 ctkochan22

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

App - Unable to scroll the PDF attachment with 2 pages (pay on 22/6)
4 participants
@rdjuric @OSBotify @iwiznia @ctkochan22