[HOLD for payment 2024-04-05] Convert internal PDF preview into a public library

[rezkiy37]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open an initial issue - [HOLD for payment 2023-08-21] [HOLD for payment 2023-08-17] [$1000] mweb - Chat - Conversation freezes when sending a large PDF file #19918
2. Read a proposal - [HOLD for payment 2023-08-21] [HOLD for payment 2023-08-17] [$1000] mweb - Chat - Conversation freezes when sending a large PDF file #19918 (comment)
3. Read important comments, starts from the next one- [HOLD for payment 2023-08-21] [HOLD for payment 2023-08-17] [$1000] mweb - Chat - Conversation freezes when sending a large PDF file #19918 (comment)

Expected Result:

Actually, the Expensify app already uses react-pdf to preview PDF files. However, it does not support the large PDF files. The idea is to create, own and maintain a package. It should support the large PDF files. It is helpful to the community, if the package can be widely used for performance improved PDF viewer. The Expensify application uses the package instead of an internal implementation.

Actual Result:

This is an internal implementation. Recently, we fixed and improved the previewing of large PDF files, directly updating our own codebase - #22760.

Workaround:

The user still can use Expensify without this being fixed.

Area issue was found in:

I was found during the investigating/fixing the next issue - #19918.

Failed WCAG checkpoints

N/A

User impact:

It does not affect the user. It has an impact on the community.

Suggested resolution:

1. Create a repository within the Expensify organisation.
2. Implement own PDF previewer based on react-pdf and react-window.
3. Add and use the previewer in the Expensify app.
4. Publish the package.

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: N/A
Reproducible in staging?: N/A
Reproducible in production?: N/A
Email or phone of affected tester (no customers): N/A
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
N/A
Expensify/Expensify Issue URL: N/A
Issue reported by: Mykhailo Kravchenko - Callstack
Slack conversation: N/A

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f329b3f580153b04
• Upwork Job ID: 1683525690767454208
• Last Price Increase: 2023-07-24

Issue Owner

Current Issue Owner: @anmurali

[rezkiy37]

Hi, @allroundexperts!
In order of our agreement, could you please:

1. Create a repository within the Expensify organisation. We will work there.
2. Describe how we create own package step by step.

Thank you!

[allroundexperts]

Hi @rezkiy37!
My understanding was that the package would be created in my git account. I don't really have access to create a repository inside the Expensify organisation. We might want to confirm this again in the bug report?

[AndrewGable]

What is the repository name? I can get one made with correct permissions. Thank you

[rezkiy37]

What is the repository name? I can get one made with correct permissions. Thank you

@AndrewGable, are you able to create a repository within Expensify organisation?

[rezkiy37]

We haven't considered names yet. I believe react-fast-pdf would be cool one.

[allroundexperts]

Agreed!

[melvin-bot]

Triggered auto assignment to @nathanmetcalf (ring0), see https://stackoverflow.com/c/expensify/questions/6102 for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01f329b3f580153b04

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @s77rt (Internal)

[AndrewGable]

@nathanmetcalf - Please create an public Expensify repository named react-fast-pdf.

We will publish under npm under @Expensify/react-fast-pdf. Please ping me once it's ready to publish on npm.

cc @puneetlath @mountiny

[nathanmetcalf]

Can I please have a licence type, and a Description for this new repo?

[nathanmetcalf]

Otherwise, I've created it. I'm not sure what permissions if any we need.
https://github.com/Expensify/react-fast-pdf

[rezkiy37]

and a Description for this new repo?

@nathanmetcalf, do you mean this? If yes, I will prepare and post text.

[AndrewGable]

MIT license please, can we leave the description blank for now? If not, just saying: react-fast-pdf seems fine for now.

[rezkiy37]

@nathanmetcalf, I am not able to fork the repo, because it is empty entirely. Could someone push at lease 1 commit there?

[AndrewGable]

Looks like we will have to wait for @nathanmetcalf to fix this as I don't have permission to push to main or my own branch.

[nathanmetcalf]

What permissions should be granted to this repo? I don't think we want to leave it open to everyone?

[rezkiy37]

What permissions should be granted to this repo? I don't think we want to leave it open to everyone?

External developers would work with own forks. Internal developers would follow your internal rules and approaches.
That's my understanding. Please correct me if I am wrong 🙂

[AndrewGable]

@nathanmetcalf - Same permissions as App or Bedrock. Open source read, write for any internal employees.

[nathanmetcalf]

These are the groups in App:

And these are the groups from that list that I think we would have needed. Do we need to add any further?

[AndrewGable]

We will probably need the Expensify.cash OSS bots too.

[AndrewGable]

@nathanmetcalf - Can you change default branch to main?

https://github.com/Expensify/react-fast-pdf/tree/main - there is a commit now, try to fork please @rezkiy37

[rezkiy37]

@nathanmetcalf - Can you change default branch to main?

https://github.com/Expensify/react-fast-pdf/tree/main - there is a commit now, try to fork please @rezkiy37

I've forked successfully 👍
Thank you!

[nathanmetcalf]

Thanks Andrew, I've added Expensify.cash OSS bots, and set the default branch to main.

Please reopen this if there is anything further you need :)

[melvin-bot]

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

[rezkiy37]

The last exected PR was merged. We finally connected the package to the app.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.57-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Connect react-fast-pdf #33434

If no regressions arise, payment will be issued on 2024-04-05. 🎊

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment (Needs manual offer from BZ)
• @allroundexperts requires payment through NewDot Manual Requests
• @rezkiy37 does not require payment (Contractor)

[melvin-bot]

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@s77rt / @allroundexperts] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@anmurali] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[s77rt]

@allroundexperts Will you handle the above checklist as you reviewed the linked PR?

[allroundexperts]

This was a new feature so a checklist is not needed here. However, I propose the following regression test:

1. On mweb Safari, go to any chat and upload a very large pdf file (More than 10 MB eg 10840-002.pdf).
2. Open the pdf and verify that you can scroll to the end without any lags / freezes.

Do we 👍 or 👎 ?

[mountiny]

@allroundexperts Can you define the minimum size for the PDF? Or to be more specific to test this case. Thanks

[allroundexperts]

@mountiny Updated!

[allroundexperts]

I reviewed the following PRs as part of this ticket:

Expensify/react-fast-pdf#1
Expensify/react-fast-pdf#2
Expensify/react-fast-pdf#8
Expensify/react-fast-pdf#12
Expensify/react-fast-pdf#14

#33434

The first two I think were during the time we were paying 1k per bug / review.

[mountiny]

@allroundexperts @rezkiy37 Just to make sure are we all done here in terms of the steps we planned for the library?

if this ready to pay out and close?

[allroundexperts]

I think yes, we're done here. @rezkiy37 can confirm though.

[rezkiy37]

Yes. The idea was to share the bugfix with the community. The package inherits the previous app behavior with all fixes and improvements. I would say that the 1st version of react-fast-pdf is done. All further changes we should consider are the 2nd version of the package.

We are done with this issue.

cc @mountiny @allroundexperts

[mountiny]

Thank you, the payment summary would be:

Expensify/react-fast-pdf#1 - $1000
Expensify/react-fast-pdf#2 - $1000
Expensify/react-fast-pdf#8 - $500
Expensify/react-fast-pdf#12 - $500
Expensify/react-fast-pdf#14 - $500
#33434 - $500

So $4000 to @allroundexperts for their work on this project with @rezkiy37 its taken a long time and many hours of testing and review so I think this is reasonable.

[mountiny]

cc @anmurali

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @s77rt paid $(AMOUNT) via Upwork (LINK)
• Reviewer: @allroundexperts owed $250 via NewDot
• Contributor: @rezkiy37 is from an agency-contributor and not due payment

BugZero Checklist (@anmurali)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1683525690767454208/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[anmurali]

Per #22998 (comment)

• @allroundexperts $4,000 on ND (approved)
• @s77rt $500 for Connect react-fast-pdf #33434 (Offer)

[s77rt]

I'm not due payment here. This can be closed

[mountiny]

I believe @allroundexperts will be handled in newdot then, so closing, thanks everyone!

[JmillsExpensify]

$4,000 approved for @allroundexperts