[PAID] [$1000] Web- Attachment - Up and down arrow key does not work for scrolling PDF file.

[kbecciv]

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. Go to staging.new.expensifail.com
2. Log in with expensifail account
3. Select any user
4. Click the plus button to the left of the message input, and select "Add attachment"
5. Select a password-protected PDF
6. Verify that the password form appears
7. Enter the password and Click Confirm
8. Use up and down key for scrolling the issue

Expected Result:

Up and down arrow key works for scrolling PDF file.

Actual Result:

Up and down arrow key does not work for scrolling PDF file.

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.97.2

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Recording.1072.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @pecanoro (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[pecanoro]

Yeah, I was able to reproduce this, it happens mostly when previewing the attachment before sending it.

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[JmillsExpensify]

Missed this in the end of offshore shuffle. Posting now.

[JmillsExpensify]

Upwork job created here: https://www.upwork.com/jobs/~0135398a55eee993ef. We're open for proposals.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

[melvin-bot]

Triggered auto assignment to @neil-marcellini (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[andrewlor]

Proposal

Problem

The scrollable pdf container is not focusable because tabIndex=-1 by default. In order to scroll using the arrow keys, you have to click the container first.

Solution

In order to programatically focus the pdf container, we'll add a ref. Then in the callback for successfully loading the pdf, set the tabIndex=0 and call .focus().

App/src/components/PDFView/index.js

Lines 48 to 54 in ddd34f8

	onDocumentLoadSuccess({numPages}) {
	this.setState({
	numPages,
	shouldRequestPassword: false,
	isPasswordInvalid: false,
	});
	}

    onDocumentLoadSuccess({numPages}) {
        this.setState({
            numPages,
            shouldRequestPassword: false,
            isPasswordInvalid: false,
        });

+       this.pdfContainer.tabIndex = 0;
+       this.pdfContainer.focus();
    }

App/src/components/PDFView/index.js

Lines 120 to 156 in ddd34f8

	return (
	<View style={outerContainerStyle}>
	<View
	style={pdfContainerStyle}
	onLayout={event => this.setState({windowWidth: event.nativeEvent.layout.width})}
	>
	<Document
	loading={<FullScreenLoadingIndicator />}
	file={this.props.sourceURL}
	options={{
	cMapUrl: 'cmaps/',
	cMapPacked: true,
	}}
	externalLinkTarget="_blank"
	onLoadSuccess={this.onDocumentLoadSuccess}
	onPassword={this.initiatePasswordChallenge}
	>
	{_.map(_.range(this.state.numPages), (v, index) => (
	<Page
	width={pageWidth}
	key={`page_${index + 1}`}
	pageNumber={index + 1}
	/>
	))}
	</Document>
	</View>
	{this.state.shouldRequestPassword && (
	<PDFPasswordForm
	onSubmit={this.attemptPdfLoad}
	onPasswordUpdated={() => this.setState({isPasswordInvalid: false})}
	isPasswordInvalid={this.state.isPasswordInvalid}
	shouldAutofocusPasswordField={!this.props.isSmallScreenWidth}
	onPasswordFieldFocused={this.toggleKeyboardOnSmallScreens}
	/>
	)}
	</View>
	);

                <View
                    style={pdfContainerStyle}
                    onLayout={event => this.setState({windowWidth: event.nativeEvent.layout.width})}
+                   ref={node => this.pdfContainer = node}
                >

Result

Screen.Recording.2022-09-14.at.6.27.21.PM.mp4

[Santhosh-Sellavel]

Reviewing this now!

[Santhosh-Sellavel]

@andrewlor

When you open a pdf that is already received, after clicking on it we are able to scroll using the up/arrow. But why isn't working while sending alone?

Can you check on that, we don't want to auto set focus.

[melvin-bot]

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

• makes the PDF container focusable to provide accessibility #12391

If no regressions arise, payment will be issued on 2022-11-22. 🎊

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[JmillsExpensify]

Lots going on in the open source world right now and the priority of this issue has slipped for me. I'll work on closing the loop as soon as I can, at the latest when the regression period ends.

[JmillsExpensify]

Circling back to pay out contributors. I still need to dig into the regression test, but let me first quickly issue payments since no regressions have arisen. Issue is not eligible for the 3 day merge bonus.

[0xmiros]

@s77rt said it's regression here
Also slack discussion regarding this regression here

[JmillsExpensify]

Yes, we've discussed this regression above as well.

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[Santhosh-Sellavel]

not overdue

[JmillsExpensify]

Closing the loop on this today. Apologies again for the delay on the regression tests!

[JmillsExpensify]

Alrighty, closing the loop. I created/linked the regression test. @neil-marcellini would you mind taking a look in the linked issue? I've added the up/down navigation to one of the existing tests, though we've since decided that tab navigation isn't officially supported, so I didn't add anything around that case. Closing the issue now that the regression test is added.

[sobitneupane]

@JmillsExpensify Am I eligible for payment for reporting the regression?

[JmillsExpensify]

While we don''t consider these bugs moving forward, as TAB is an accessibility feature we haven't built, yes I think that's fair game for a reporting bonus. Upwork job is here: https://www.upwork.com/jobs/~018a6911f84f687773.

[sobitneupane]

@JmillsExpensify Proposal submitted.

[JmillsExpensify]

Offer sent.

[JmillsExpensify]

You're all set. Paid out!

[dhairyasenjaliya]

@JmillsExpensify regarding these accessibility bugs I have reported similar 2 bugs/regression #12058 is related to this bug on the same component, am I eligible for reporting those?

#12058
#12010

[JmillsExpensify]

Please take that question to open source in Slack. However for clarity we only compensate for merged PRs. This specific issue was reported by our regression testing partner.