fix: IOU Scan - In dark mode, the damaged PDF - file is barely visible.

[Krishna2323]

Details

Fixed Issues

$ https://github.com/Expensify/App/issues/39356 PROPOSAL: https://github.com/Expensify/App/issues/39356#issuecomment-2029731489

Tests

1. Open FAB mentu > submit expense > scan
2. Upload corrupt pdf attached below > Select any participant
3. Verify there is a placeholder is shown with receipt slash component on confirmation page ? Submit expense
4. Verify transactions preview also shows receipt slash component
5. Click on transactions preview and verify receipt slash component is also displayed on transaction preview
6. Open transaction details page and verify receipt slash component in place of receipt place holder

• [x] Verify that no errors appear in the JS console

Offline tests

1. Open FAB mentu > submit expense > scan
2. Upload corrupt pdf attached below > Select any participant
3. Verify there is a placeholder is shown with receipt slash component on confirmation page ? Submit expense
4. Verify transactions preview also shows receipt slash component
5. Click on transactions preview and verify receipt slash component is also displayed on transaction preview
6. Open transaction details page and verify receipt slash component in place of receipt place holder

QA Steps

1. Open FAB mentu > submit expense > scan
2. Upload corrupt pdf attached below > Select any participant
3. Verify there is a placeholder is shown with receipt slash component on confirmation page ? Submit expense
4. Verify transactions preview also shows receipt slash component
5. Click on transactions preview and verify receipt slash component is also displayed on transaction preview
6. Open transaction details page and verify receipt slash component in place of receipt place holder

• [x] Verify that no errors appear in the JS console

PR Author Checklist

• [x] I linked the correct issue in the ### Fixed Issues section above
• [x] I wrote clear testing steps that cover the changes made in this PR
  • [x] I added steps for local testing in the Tests section
  • [x] I added steps for the expected offline behavior in the Offline steps section
  • [x] I added steps for Staging and/or Production testing in the QA steps section
  • [x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • [x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • [x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• [x] I included screenshots or videos for tests on all platforms
• [x] I ran the tests on all platforms & verified they passed on:
  • [x] Android: Native
  • [x] Android: mWeb Chrome
  • [x] iOS: Native
  • [x] iOS: mWeb Safari
  • [x] MacOS: Chrome / Safari
  • [x] MacOS: Desktop
• [x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• [x] I followed proper code patterns (see Reviewing the code)
  • [x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • [x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • [x] I verified that comments were added to code that is not self explanatory
  • [x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • [x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • [x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • [x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • [x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • [x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • [x] I verified the JSDocs style guidelines (in STYLE.md) were followed
• [x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• [x] I followed the guidelines as stated in the Review Guidelines
• [x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• [x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• [x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• [x] I verified that if a function's arguments changed that all usages have also been updated correctly
• [x] If any new file was added I verified that:
  • [x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• [x] If a new CSS style is added I verified that:
  • [x] A similar style doesn't already exist
  • [x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• [x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• [x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• [x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• [x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • [x] I verified that all the inputs inside a form are aligned with each other.
  • [x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• [x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• [x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

https://github.com/Expensify/App/assets/85894871/a00a0769-6e56-4a9e-8fac-c7d8abbfd110

Android: mWeb Chrome

https://github.com/Expensify/App/assets/85894871/a4dd89e3-4ec8-445e-a134-6174efbc771e

iOS: Native

https://github.com/Expensify/App/assets/85894871/6207d5c5-20a6-4983-b358-918caaae763a

iOS: mWeb Safari

https://github.com/Expensify/App/assets/85894871/d6ecb4f8-67e9-4055-9b56-2db83adeaaea

MacOS: Chrome / Safari

https://github.com/Expensify/App/assets/85894871/02c98eb5-a5c1-425d-a9a8-f52336f0ae4a

MacOS: Desktop

https://github.com/Expensify/App/assets/85894871/320ace27-7210-4c8c-b4c0-a8f89a37d29e

[melvin-bot[bot]]

@jayeshmangwani Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[Krishna2323]

@jayeshmangwani, when selecting a corrupt pdf file on native devices we don't get error and empty white background is shown. I'm looking for a solution for that.

[jayeshmangwani]

@Krishna2323 When you push the changes for native devices, please ping me.

[Krishna2323]

@jayeshmangwani, I have updated the code for native devices, can you pls check and let me know, I haven't updated the recordings for native yet.

Demo for for native devices:

https://github.com/Expensify/App/assets/85894871/11a1d222-d8fb-47d3-82c8-e40e1af20b28

[jayeshmangwani]

@Expensify/design Please confirm the styling of the Failed to load PDF file. text?

[shawnborton]

I would think instead of showing text like we are, we'd show our default fallback receipt view, something like this that @dannymcclain had suggested previously:

Though I think I would change out the icon to use some kind of broken image icon, something like this but in our brand style:

[dannymcclain]

@shawnborton I thought in a different issue we had settled on not allowing someone to upload a corrupted image/PDF? We show the standard modal that says "Error uploading attachment..." This was the mock for that:

Am I misremembering something?

[shawnborton]

You are definitely right. So yeah, that begs the question, how did we trigger this particular "Failed to load PDF" scenario?

[jayeshmangwani]

@shawnborton @dannymcclain To reproduce this issue, I am following this steps:

1. Go to workspace chat > + > Split Expense > Scan > Upload a corrupt pdf

https://github.com/Expensify/App/assets/35371050/1ca7f71b-8b01-4981-9923-150f29d21817

Corrupt pdf

[corrupted.pdf](https://github.com/Expensify/App/files/15067877/corrupted.pdf)

[Krishna2323]

@dannymcclain @shawnborton, Yep, you're correct here. I worked on preventing corrupted images from being uploaded, but that was specifically for images. There might be another GitHub issue addressing prevention of corrupt PDFs from being uploaded. PR.

[Krishna2323]

Here is the issue for preventing corrupted pdf's from being uploaded.

[shawnborton]

Yeah, I feel like a much better solution is to prevent a corrupted PDF from being uploaded in the first place.

[jayeshmangwani]

@hayata-suenaga @shawnborton If we are going to prevent a corrupted PDF from being uploaded, then we can close this PR and close/hold the issue in favor of https://github.com/Expensify/App/issues/34032

[shawnborton]

That's where my head is at... it seems like we won't ever be able to get to this scenario once we prevent corrupted images/files from being uploaded.

[trjExpensify]

it seems like we won't ever be able to get to this scenario once we prevent corrupted images/files from being uploaded.

I don't think we know the image/file is corrupt purely on the client, so isn't it possible you'll run into this when uploading a corrupt file offline?

[shawnborton]

Hmm good question, who can confirm that?

If that's the case, I like doing something like my suggestion here.

[dannymcclain]

If that's the case, I like doing something like my suggestion https://github.com/Expensify/App/pull/40607#issuecomment-2070156624.

Yeah I agree. @shawnborton do you think any of these icons would work for that? (Receipt Slash and Document Slash are already in the system—Image Slash is one that I just made based off of the Image icon and Receipt Slash icon.)

[shawnborton]

Yeah I think any of these would be great! I think I like doing something super generic in case this is a pattern we'd repeat not only for PDF-specific receipts but for all receipts in general that are corrupt. So that being said, I lean towards the first option?

[dannymcclain]

Yeah first option makes sense to me—that way we could use it for any file type of receipt that borked.

[hayata-suenaga]

I don't think we know the image/file is corrupt purely on the client, so isn't it possible you'll run into this when uploading a corrupt file offline?

I'm not familiar with how we're handling the PDF upload on the client side, but we try to display a preview of the PDF before the user attempts to send it. If the PDF is corrupted, we know that when we try to display the preview.

@Krishna2323, could you confirm that you seem to be working on the issue to prevent the corrupt PDF from being uploaded?

Regardless, I think it's a good idea to still implement the fallback view if the attachment is corrupted though.

[Krishna2323]

@hayata-suenaga, I've worked on preventing corrupt images from being uploaded, and it seems to be functioning well across all platforms. As for the PDF issue, it's slated to be addressed in this issue. The proposal selected to resolve the problem doesn't appear to offer a straightforward solution for detecting corrupt PDFs across all platforms. Therefore, I agree with your suggestion to proceed with implementing the fallback view. Additionally, I've noticed that some PDFs preview correctly on the confirmation list but then display an error message stating that the PDF can't be loaded.

You can try with this pdf: Bug6460367_1713968986208.class-9-maths-chapter-10-solutions-2024-02-08_22_17_00.194.pdf

This works fine until we submit the request and go to details page but as soon as we refresh it shows Failed to load PDF file.

[Krishna2323]

Ok, so I guess, we agree on showing fallback for corrupted files, will start implementing the changes today and update.

[hayata-suenaga]

Ok, so I guess, we agree on showing fallback for corrupted files, will start implementing the changes today and update.

Yes sorry if that wan't clear 🙇 thank you for pushing the PR forward 👍

[Krishna2323]

@dannymcclain, I don't think we have Receipt Slash in the app, can you pls share it.

[dannymcclain]

I don't think we have Receipt Slash in the app, can you pls share it.

@Krishna2323 No prob!

Receipt Slash.svg.zip

[Krishna2323]

Code changes are done. @dannymcclain @shawnborton, can we confirm what width and height size we should use for the ReceiptSlash icon?

With 72px:

Receipt doc size:

[shawnborton]

Hmm I could have sworn we already have a pattern in place for receipt placeholders, like these:

Can we match that size? From Figma, it looks like they are at 80x80.

[Krishna2323]

@shawnborton @dannymcclain @hayata-suenaga @jayeshmangwani, while recording videos for recent changes, I found that we can prevent users from uploading corrupt PDFs, and it works on all platforms. There will be a minor difference in corrupt image detection and corrupt PDF detection: for images, we show the alert modal immediately after the user selects it, and for PDFs, we will show it on the confirmation page, just like we show the alert for protected PDFs on the confirmation page. I don't think that's a very major inconsistency, so I have already updated the code for that. The fallback is also implemented in case a corrupt PDF passes the check, but that will be in very rare cases.

https://github.com/Expensify/App/assets/85894871/716891b9-0e5d-4649-a612-cdda525ce25e

https://github.com/Expensify/App/assets/85894871/1f3564cf-46a5-4888-a09b-7c8907d15ef1

https://github.com/Expensify/App/assets/85894871/c0a85e3b-96cd-438a-bbb0-591537b852ee

[Krishna2323]

@jayeshmangwani, you can start the review.

[shawnborton]

Two questions:

Why can't we detect the corrupted PDF immediately, the same way we do for images?

How did you get a camera view like this where the capture button is off to the right? Looks like we're missing a flash icon?

[dannymcclain]

Why can't we detect the corrupted PDF immediately, the same way we do for images?

Also curious about this. It feels pretty weird to drop the user back at the contact selector when they acknowledge the warning. They don't need to pick a new contact, they need to upload a different file. So that feels pretty strange to me.

[Krishna2323]

@dannymcclain @shawnborton, we can push the user to file selector page instead of participants selector but we can't detect the pdf immediately. For images we use react-native-image-size package which works on all platforms but we don't have any package to detect corrupt pdf's. We first need to render it with different libraries for native and web and if the render fails we get the error. You can try uploading a protected pdf, you won't get any alert until you reach the confirmation page.

[shawnborton]

Hmm yeah, I'm a bit stumped so I would be curious for what our engineering team thinks.

I think the reality is, we haven't had a single customer ever report this corrupted PDF flow. This was a super edge case flow reported by our QA team. So I'm hesitant to spin the wheels too much here, given that I think this is not going to be any kind of mainline flow that we should optimize for.

If what you are proposing is the best we can do with our current limitations, I think it's probably our best bet.

[hayata-suenaga]

@dannymcclain @shawnborton, we can push the user to file selector page instead of participants selector but we can't detect the pdf immediately. For images we use react-native-image-size package which works on all platforms but we don't have any package to detect corrupt pdf's. We first need to render it with different libraries for native and web and if the render fails we get the error. You can try uploading a protected pdf, you won't get any alert until you reach the confirmation page.

My knowledge of PDF flow is also very limited. But one question. We still display the corrupt PDF on the front end before uploading the PDF to the backend, right? If the file is corrupted, why the front end can display the file?

[Krishna2323]

@hayata-suenaga, the library attempts to display the provided pdf, but when it fails, it throws an error and shows 'Failed to load pdf file' instead of the pdf on the frontend. Corrupt pdf isn't displayed on the frontend in any case, only the error message is shown. What I did is captured that thrown error and displayed the alert modal.

[Krishna2323]

@jayeshmangwani, bump for review.

[hayata-suenaga]

bumped them on Slack

[jayeshmangwani]

@Krishna2323 Remove HOLD from the PR title and resolve the conflicts In local I am getting conflicts with the latest main branch

[jayeshmangwani]

resolve the conflicts In local I am getting conflicts with the latest main branch

@Krishna2323 bump for this comment

[Krishna2323]

resolve the conflicts

@jayeshmangwani, on it.

[Krishna2323]

@jayeshmangwani, conflicts resolved.