Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] Web - Chat - You can't scroll the PDF right after opening it with the "down arrow" button #34816

Closed
1 of 6 tasks
lanitochka17 opened this issue Jan 19, 2024 · 80 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jan 19, 2024

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.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:

  1. Navigate to a chat
  2. Upload a PDF
  3. Open it
  4. 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
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6347791_1705680293652.bandicam_2024-01-19_13-44-51-560.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~011e97d8eae76876e6
  • Upwork Job ID: 1748379977514749952
  • Last Price Increase: 2024-01-26
  • Automatic offers:
    • hungvu193 | Reviewer | 0
    • abzokhattab | Contributor | 0
Issue OwnerCurrent Issue Owner: @JmillsExpensify
Issue OwnerCurrent Issue Owner: @JmillsExpensify
@lanitochka17 lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 19, 2024
Copy link

melvin-bot bot commented Jan 19, 2024

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

Copy link

melvin-bot bot commented Jan 19, 2024

Job added to Upwork: https://www.upwork.com/jobs/~011e97d8eae76876e6

@melvin-bot melvin-bot bot changed the title Web - Chat - You can't scroll the PDF right after opening it with the "down arrow" button [$500] Web - Chat - You can't scroll the PDF right after opening it with the "down arrow" button Jan 19, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 19, 2024
Copy link

melvin-bot bot commented Jan 19, 2024

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

@abzokhattab
Copy link
Contributor

abzokhattab commented Jan 19, 2024

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

Screen.Recording.2024-01-19.at.7.12.15.PM.mov

@suneox
Copy link
Contributor

suneox commented Jan 19, 2024

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...

  1. Update this line
    this.setListAttributes = this.setListAttributes.bind(this);
+   this.pdfDocumentRef = React.createRef(null);
  1. When document setListAttributes we will store the ref
    setListAttributes(ref) {
        if (!ref) {
            return;
        }
+       this.pdfDocumentRef.current = ref;
        ...
    }
  1. 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

Screen.Recording.2024-01-20.at.01.31.12.mov

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

@JeremyCroff
Copy link

JeremyCroff commented Jan 19, 2024

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.

@Krishna2323
Copy link
Contributor

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.

@abzokhattab
Copy link
Contributor

abzokhattab commented Jan 21, 2024

@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

@sofi-a
Copy link

sofi-a commented Jan 21, 2024

@abzokhattab It didn't work for me without the timer and I set the duration randomly. It can be decreased if necessary.

@sofi-a
Copy link

sofi-a commented Jan 21, 2024

@abzokhattab you're right, calling ref.focus() after the ref.tabIndex = -1 statement works 👍.

@sofi-a
Copy link

sofi-a commented Jan 21, 2024

I'll remove my proposal since it's redundant.

@melvin-bot melvin-bot bot added the Overdue label Jan 22, 2024
@alexpensify
Copy link
Contributor

@situchan - any update here regarding these proposals?

@situchan
Copy link
Contributor

reviewing

@melvin-bot melvin-bot bot removed the Overdue label Jan 22, 2024
@jeremy-croff
Copy link
Contributor

jeremy-croff commented Jan 22, 2024

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)

@alexpensify
Copy link
Contributor

Sounds like a plan, keep us posted @situchan. Thanks!

@situchan
Copy link
Contributor

I think we should hold this for:

Copy link

melvin-bot bot commented Jan 26, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@alexpensify alexpensify changed the title [$500] Web - Chat - You can't scroll the PDF right after opening it with the "down arrow" button [Hold - EApp #32800] [$500] Web - Chat - You can't scroll the PDF right after opening it with the "down arrow" button Jan 26, 2024
@alexpensify
Copy link
Contributor

Thanks, moving this to weekly and putting it on hold.

@melvin-bot melvin-bot bot added the Overdue label Jan 29, 2024
@situchan
Copy link
Contributor

Still holding

@alexpensify
Copy link
Contributor

Heads up, I will be offline until Tuesday, April 23, 2024, and will not actively watch over this GitHub during that period.

If anything urgent is needed here, please ask for help in the #expensify-open-source Slack Room-- thanks!

@alexpensify
Copy link
Contributor

@hungvu193 - catching up from being OOO, can we get an update on what's remaining here? Thanks!

@hungvu193
Copy link
Contributor

@hungvu193 - catching up from being OOO, can we get an update on what's remaining here? Thanks!

The PR was reverted since it was linked to a DB and we couldn't figure out the way to fix it.

@alexpensify
Copy link
Contributor

Got it, so is the issue still present, or did it self-resolve? Thanks!

@hungvu193
Copy link
Contributor

Got it, so is the issue still present, or did it self-resolve? Thanks!

Unfortunately it's still present

@alexpensify

This comment was marked as outdated.

@alexpensify
Copy link
Contributor

alexpensify commented May 10, 2024

This week, I've been focused on Wave Collect resources so I'll start a discussion on Monday.

@alexpensify alexpensify removed their assignment May 17, 2024
@alexpensify alexpensify added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels May 17, 2024
Copy link

melvin-bot bot commented May 17, 2024

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 17, 2024
@alexpensify alexpensify added Weekly KSv2 and removed Daily KSv2 labels May 17, 2024
@alexpensify
Copy link
Contributor

alexpensify commented May 17, 2024

Heads up, I will be offline until Tuesday, May 28, 2024, and will not actively watch over this GitHub during that period.

@JmillsExpensify— Next week, if the site is in a more stable spot, please start a discussion in the Open Source Slack room about whether we should find another solution here or close this GH. I waited to have that conversation since the focus has been on higher-priority issues, so I appreciate your help while I'm OOO. Thanks!

Here is where we are at:

The PR was reverted since it was linked to a DB and we couldn't figure out the way to fix it.

@JmillsExpensify
Copy link

I think we should just close it. If someone is passionate, please raise a discussion in the Open Source Slack room and re-open when we've agreed on a solution.

@hungvu193
Copy link
Contributor

Since I and @abzokhattab already worked on the issue. Can we request a compensation here? @JmillsExpensify

@JmillsExpensify
Copy link

What do you think is reasonable?

@hungvu193
Copy link
Contributor

I think half of the bounty.

@JmillsExpensify
Copy link

That works for me, though in case anyone happens upon this decision in the future, it's a one-off and not indicative of how we'll approach cases like this in the future. In any case:

@JmillsExpensify
Copy link

Contracts are paid out via Upwork. @abzokhattab also please check the message I sent you in Upwork, thanks!

@abzokhattab
Copy link
Contributor

Alright thanks @JmillsExpensify .. i have refunded 250$ since i was already paid few days ago. please let me know if you have any other requests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests