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

Updated Readium version #261

Merged
merged 1 commit into from
Nov 20, 2023
Merged

Updated Readium version #261

merged 1 commit into from
Nov 20, 2023

Conversation

nunommts
Copy link
Contributor

@nunommts nunommts commented Nov 17, 2023

What's this do?
This PR updates the app to support the latest version of Readium

Why are we doing this? (w/ JIRA link if applicable)
There's a feature request to update the Readium version

How should this be tested? / Do these changes have associated tests?
Open the Palace app
Open any Ebook
Confirm everything is working
Return and open any PDF
Confirm everything is working

Dependencies for merging? Releasing to production?
This PR also needs to be merged

Have you updated the changelog?
Yes

Has the application documentation been updated for these changes?
No

Did someone actually run this code to verify it works?
Tested by @nunommts

@io7m
Copy link
Contributor

io7m commented Nov 20, 2023

This looks fine to me, but I must admit that I have no idea what's going on with our PDF support. How is Readium involved? I thought we used some other third party library.

@nunommts
Copy link
Contributor Author

@io7m We're using their PdfParser file. One of the main changes between the previous Readium version and this one is that the PdfParser now doesn't accept a constructor with no PDFFactory argument that would use their "default" document factory, so I had to import the code they're using on it.

@nunommts nunommts merged commit 34455aa into main Nov 20, 2023
1 check passed
@nunommts nunommts deleted the feature/update-readium-version branch November 20, 2023 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants