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.

Sign up for GitHub

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

Jump to bottom

pdfjs-dist update #6948

Open
wants to merge 22 commits into
base: development
Choose a base branch
from
Open

pdfjs-dist update #6948

wants to merge 22 commits into from

Conversation

Zasa-san
Copy link
Collaborator

fixes #6920

PR checklist:

  • Update READ.me ?
  • Update API documentation ?

QA checklist:

  • Smoke test the functionality described in the issue
  • Test for side effects
  • UI responsiveness
  • Cross browser testing
  • Code review

@Zasa-san Zasa-san marked this pull request as ready for review June 30, 2024 21:08
@Zasa-san Zasa-san mentioned this pull request Jul 1, 2024
Closed
@Zasa-san Zasa-san marked this pull request as draft July 1, 2024 13:01
@Zasa-san Zasa-san requested a review from mfacar July 2, 2024 17:09
@Zasa-san Zasa-san marked this pull request as ready for review July 2, 2024 17:09
@Zasa-san Zasa-san mentioned this pull request Jul 2, 2024
Open
@Zasa-san
Copy link
Collaborator Author

Zasa-san commented Jul 2, 2024 •
edited
Loading

I'm avoiding testing a specific behavior in the cypress/e2e/attachments.cy.ts test since the modal to upload files does not consistently close. Since this PR is meant to address other issues and not the supporting files behavior I think it's best to finish it, and then address the general problems with the modal in #6963

In this PR I will just migrate the test to cypress and cover what the puppeteer test was already doing with a few additions. The behaviors that are not tested are uploading a supporting file and adding a supporting file via URL. This was not tested in the original test either.

mfacar
mfacar approved these changes Jul 2, 2024
View reviewed changes
Copy link
Collaborator

@mfacar mfacar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice improvements!

I left a couple of minor notes in the code for your consideration.

cypress/cypress.d.ts Show resolved Hide resolved
cypress/e2e/references.cy.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfacar mfacar mfacar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade PDF js
2 participants
@Zasa-san @mfacar