App
App copied to clipboard
[$500] Web - Chat - You can't scroll the PDF right after opening it with the "down arrow" button
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.28.0 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: Applause - Internal Team Slack conversation:
Action Performed:
- Navigate to a chat
- Upload a PDF
- Open it
- Press the "down arrow" keyboard button
Expected Result:
I should be able to use the arrows without clicking
Actual Result:
You can't scroll the PDF right after opening it with the "down arrow" button. You need to click once for the function to work
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [ ] Android: Native
- [ ] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/78819774/c8f29f90-636c-45fa-b250-aaa508296136
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~011e97d8eae76876e6
- Upwork Job ID: 1748379977514749952
- Last Price Increase: 2024-01-26
Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Job added to Upwork: https://www.upwork.com/jobs/~011e97d8eae76876e6
Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)
Proposal
Please re-state the problem that we are trying to solve in this issue.
You can't scroll the PDF right after opening it with the down or up buttons
What is the root cause of that problem?
the document is not focused after it's loaded
What changes do you think we should make in order to solve the problem?
we need to call the focus function in the setListAttributes this function is called in the outerRef of the List component
ref.focus();
FYI as you can notice, the list component is shown after this loader indicator is removed
POC
https://github.com/Expensify/App/assets/59809993/336ed375-b60f-4cee-a171-6b0476cfdce3
Proposal
Please re-state the problem that we are trying to solve in this issue.
Web - Chat - You can't scroll the PDF right after opening it with the "down arrow" button
What is the root cause of that problem?
After document is load success we don't focus to the document so we can't scroll it with the keyboard
What changes do you think we should make in order to solve the problem?
We will store the document ref and focus on it after document loaded success to avoid while document still loading user can click anywhere ST like download button...
- Update this line
this.setListAttributes = this.setListAttributes.bind(this);
+ this.pdfDocumentRef = React.createRef(null);
- When document setListAttributes we will store the ref
setListAttributes(ref) {
if (!ref) {
return;
}
+ this.pdfDocumentRef.current = ref;
...
}
- After that when document onDocumentLoadSuccess we will focus into the document
onDocumentLoadSuccess(pdf) {
const {numPages} = pdf;
+ if(this.pdfDocumentRef.current && this.pdfDocumentRef.current.focus) {
+ this.pdfDocumentRef.current.focus();
+ }
...
}
POC
https://github.com/Expensify/App/assets/11959869/c6e016af-7d75-44fc-a362-ea259e736c73
What alternative solutions did you explore? (Optional)
Instead of storing ref at PDFView we can store ref at WebPDFDocument and focus on it when the document loaded success after that continue to call onDocumentLoadSuccess
Proposal
Please re-state the problem that we are trying to solve in this issue.
A newly opened PDF cannot be scrolled with keyboard without clicking on it first
What is the root cause of that problem?
No focus is being set on the document
What changes do you think we should make in order to solve the problem?
react-pdf has built in props to handle this. ( the currently used open source package )
<PDFViewer innerRef={node => (this.iframe = node)}>
<Document onRender={() => this.iframe.focus()}>
//PDF
</Document>
</PDFViewer>
I will test the focus on a password protected document as well to ensure there's no conflicts.
What alternative solutions did you explore? (Optional)
Alternatively, you can set your focus by creating your own refs in React.
I don't think it is a bug, even in mac pdf preview and chrome pdf viewer you can't scroll without clicking on the pdf.
@sofi-a It works fine without the timer just make sure to put this ref.focus()after this line
plus there is some lagging when putting the time .. the user has to wait 1 second to use the scroll
@abzokhattab It didn't work for me without the timer and I set the duration randomly. It can be decreased if necessary.
@abzokhattab you're right, calling ref.focus() after the ref.tabIndex = -1 statement works 👍.
I'll remove my proposal since it's redundant.
@situchan - any update here regarding these proposals?
reviewing
My original.proposal is currenly hidden because my github account is currently suspended while it's being recovered. The proposal should be in an email history.
The proposal was to use the react-pdf built in props to maintain focus
Proposal
Please re-state the problem that we are trying to solve in this issue.
PDFs opened in a new tab don't focus
What is the root cause of that problem?
This is not default behaviour, and must be coded for.
What changes do you think we should make in order to solve the problem?
We should leverage the react-pdf components and it's existing props to manage a focus callback on ready state of the document
<PDFViewer innerRef={node => (this.iframe = node)}>
<Document onRender={() => this.iframe.focus()}>
// PDF implementation
</Document>
</PDFViewer>
What alternative solutions did you explore? (Optional)
Sounds like a plan, keep us posted @situchan. Thanks!
I think we should hold this for:
- https://github.com/Expensify/App/pull/32800
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Thanks, moving this to weekly and putting it on hold.
Still holding
🤦🏼 looks like my tags didn't stick last time. Moving this to Weekly!
Weekly update: On Hold
Weekly Update: On Hold
Weekly Update: On Hold
Weekly Update: @situchan it looks like #32800 is going to close. Should we close this one too or take this one off hold?
@situchan can you please update here if we should close this one too? Thanks!
I have checked this issue still happens, so i think we should focus after pdf file loaded successfully
The holding PR was closed but the root cause still remains. There's another attempt to use focus trap in https://github.com/Expensify/App/issues/36476.
@situchan - should we hold for https://github.com/Expensify/App/issues/36476 or update the focus like @suneox suggested? Thanks!
Just to clarify my proposal, this line inside the setListAttributes function is gonna be reached only when the document is loaded successfully,
as per the source code of the Document component in the react-pdf library, the children components will be only rendered when the pdf is loaded successfully which means the setListAttributes should be called after the doc is loaded.
Just to clarify my proposal, this line inside the
setListAttributesfunction is gonna be reached only when the document is loaded successfully,
Based on the code base the event for PDFDocument loaded successfully at onDocumentLoadSuccess callback, and setListAttributes at this line is only keeping the ref for the List has import from react-window at this line