[C+ Checklist Needs Completion] [$500] "Failed to load PDF file" message is not rendering in message preview

@ishpaul777

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

Version Number: 1.4.36-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @ishpaul777
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1706234706816449

Action Performed:

Upload a Corrupted pdf in any report

Expected Result:

if pdf preview fails to load "Failed to load PDF file" render

Actual Result:

Nothing renders

Workaround:

unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0163673ea563ca697f
Upwork Job ID: 1754473111912144896
Last Price Increase: 2024-02-19
Automatic offers:

getusha | Reviewer | 0

Job added to Upwork: https://www.upwork.com/jobs/~0163673ea563ca697f

@getusha

Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha (External)

@greg-schroeder

Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

Can't reproduce - on my side on Android it shows an error if I try to upload a corrupted PDF.

Can you try reproducing with this one Corrupted.PDF.pdf

Proposal

Please re-state the problem that we are trying to solve in this issue.

"Failed to load PDF file" message is not rendering in message preview.

What is the root cause of that problem?

Three is a issue with mention file here. It seems there is an issue with the provided file (index.android.js) where the PDF content is still being rendered even when attachmentCarouselPagerContext is null

What changes do you think we should make in order to solve the problem?

To address this issue, you can modify the code to check for the null value of attachmentCarouselPagerContext and display an error message accordingly. Here is an example of how you can update the code:

{attachmentCarouselPagerContext === null ? (
    <View style={[themeStyles.flex1, themeStyles.justifyContentCenter]}>
        <Text style={props.errorLabelStyles}>{translate('attachmentView.failedToLoadPDF')}</Text>
    </View>
) : (
    // same as existing code
)}

include components on top of the function

import useLocalize from '@hooks/useLocalize';
import Text from '@components/Text';

and mention below const variable on line#20

const {translate} = useLocalize();
const themeStyles = useThemeStyles();

What alternative solutions did you explore? (Optional)

You can remove the style [this.props.themeStyles.flexRow] from here

Screenshots:

Proposal

Please re-state the problem that we are trying to solve in this issue

When successfully uploading a corrupt PDF file, the message is not being rendered in the message preview.

What is the root cause of that problem?

Note

The issue is happening on botn iOS / Android native platforms.

We don't see anything on native platforms but we see "Failed to load PDF file." on web. The difference between the native / web platforms is that on native we're applying different styles to the child container View that's rendered within the Pressable when an error exists.

Where is the problem exactly ?

If we take a look within the PDFView/index.native.js we can see that the Pressable that wraps the {this.renderPDFView()} has the following styles applied: flex1, flexRow and alignSelfStretch.

While the container View rendered by renderPDFView has the following styles applied when the error is being rendered: alignItemsCenter and flex1.

These styles have a conflict which make the error View invisible due to the following reasons:

flex1 on the container View within the Pressable:
- The use of flex1 is intended to make the View expand to fill the available space in the primary axis of its parent (the Pressable). However, because the Pressable has a flexRow style, its primary axis is horizontal. This means that flex1 would only affect the width of the container View. If there's no explicit height specified for the View or the Pressable, and no other child dictates the height, the View does not have enough height to be visible.
alignItemsCenter on the child View within the Pressable:
- Being inside a parent with flexRow (the Pressable), the alignItemsCenter applied to the container View doesn't affect its width but rather its vertical alignment. This leads to the container View being centered vertically with no defined height, resulting in a zero-height View that does not display its contents.
flexRow on the parent Pressable:
- This style sets the children of the Pressable to be laid out in a horizontal line. Since flex1 on the child is trying to fill space horizontally, it doesn't provide information about how the View should expand vertically (along the cross axis).

Together, these styles result in the container View taking up all available width due to flex1 while it does not have a defined height to make it visible within the layout. The View needs distinct rules to ensure it takes up space along the vertical axis as well since flexRow on the Pressable causes the main axis to be horizontal.

Without an explicit height (or minimum height), the container View will collapse vertically and not display any of its contents, including the error View / Text when the error is present.

Important

All platforms (web / native) DO render the "Failed to load PDF file." from a code POV but we're not seeing it on native due to the conflicting styles.

What changes do you think we should make in order to solve the problem?

App/src/components/PDFView/index.native.js

Line 188 in 06c7dac

    
           style={[this.props.themeStyles.flex1, this.props.themeStyles.flexRow, this.props.themeStyles.alignSelfStretch]}

In order to make sure, as mentioned in the RCA that the Pressable's children -> the container View takes up space along the vertical axis as well since flexRow is present on the Pressable, we need to conditionally remove the flexRow style from the Pressable based on whether the error failedToLoadPDF is true.

This is the only way to have both the error comment and the error within the attachment modal render correctly, without affecting the Pressable styles when the error is not present, by keeping the flexRow style.

Changed code:

    style={[themeStyles.flex1, themeStyles.alignSelfStretch, !failedToLoadPDF && themeStyles.flexRow]}

Videos

Android: Native

Screen.Recording.2024-02-19.at.14.58.35.mov

@greg-schroeder

@greg-schroeder Friendly bump for payments 🫡

Sorry about that, I was OOO yesterday, but I'll process this today!

Processing now

@getusha

All payments complete. @getusha can you complete the checklist so we can close this out? Thanks!

@getusha

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

[@getusha] The PR that introduced the bug has been identified. Link to the PR: #21714
[@getusha] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/21714/files#r1519516670
[@getusha] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/a
[@getusha] Determine if we should create a regression test for this bug. Yes
[@getusha] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

Open any chat
Add attachment
Send corrupted pdf (https://github.com/Expensify/App/files/14166298/Corrupted.PDF.pdf)
Verify that Failed to load PDF file renders correctly.

Do we agree 👍 or 👎

Filing regression test and closing!

Navigation Menu

Repository navigation

[C+ Checklist Needs Completion] [$500] "Failed to load PDF file" message is not rendering in message preview #35808

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

Screenshots:

Proposal

Please re-state the problem that we are trying to solve in this issue

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

Videos

78 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[C+ Checklist Needs Completion] [$500] "Failed to load PDF file" message is not rendering in message preview #35808

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Feb 5, 2024

melvin-bot commented on Feb 5, 2024

melvin-bot commented on Feb 5, 2024

ikevin127 commented on Feb 5, 2024

ishpaul777 commented on Feb 5, 2024

kaushiktd commented on Feb 6, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

Screenshots:

ikevin127 commented on Feb 6, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

Videos

78 remaining items

ikevin127 commented on Mar 7, 2024

greg-schroeder commented on Mar 8, 2024

greg-schroeder commented on Mar 9, 2024

greg-schroeder commented on Mar 9, 2024

getusha commented on Mar 11, 2024

greg-schroeder commented on Mar 13, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions