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

Allow multiple PDF embeddings in one page #25

Closed
jakob-r opened this issue Oct 2, 2021 · 10 comments · Fixed by #32
Closed

Allow multiple PDF embeddings in one page #25

jakob-r opened this issue Oct 2, 2021 · 10 comments · Fixed by #32

Comments

@jakob-r
Copy link

jakob-r commented Oct 2, 2021

Problem: Embedding two PDFs in one page does not work

Solution: Use window.addEventListener("load",function(){ instead of window.onload = function(event) { and have unique IDs.

I made soma adaptions here: https://github.com/jakob-r/courseTheme/blob/main/layouts/shortcodes/pdfjs.html

If you are interested I can make a PR.

@nicfab
Copy link

nicfab commented Jan 9, 2022

Yes, it doesn't work.
It would be great to have the opportunity to publish multiple PDF files on one page or one post.

@nicfab
Copy link

nicfab commented Jan 9, 2022

Problem: Embedding two PDFs in one page does not work

Solution: Use window.addEventListener("load",function(){ instead of window.onload = function(event) { and have unique IDs.

I made soma adaptions here: https://github.com/jakob-r/courseTheme/blob/main/layouts/shortcodes/pdfjs.html

If you are interested I can make a PR.

I tried your solution, but it doesn't work because - I suppose - I have a multilingual website, and if I click on "PDF", it misses something at the beginning of the PATH.
I mean that I should see the PDF at https://domain.it/it/it/posts/PDF instead I see https://domain.it/it/posts/PDF (it miss an /it).
Do you have any solution?

@jakob-r
Copy link
Author

jakob-r commented Jan 10, 2022

Hard to say without an example. So only the download button does not work because pdf_url is not created correctly? Maybe you can change the lines here to build multilingual links correctly:
https://github.com/jakob-r/courseTheme/blob/83398a2fa9c8154c6e6a3259eb05f14b02fc3b1d/layouts/shortcodes/pdfjs.html#L6-L7

@nicfab
Copy link

nicfab commented Jan 10, 2022

Hard to say without an example. So only the download button does not work because pdf_url is not created correctly? Maybe you can change the lines here to build multilingual links correctly:
https://github.com/jakob-r/courseTheme/blob/83398a2fa9c8154c6e6a3259eb05f14b02fc3b1d/layouts/shortcodes/pdfjs.html#L6-L7

Thank you for your reply.
My Hugo website has the following architecture:

content
|_en
|_page
|_posts
|_it
|_page
|_posts

Can you help me in setting the correct path?
You can see the website online here: https://www.nicfab.it
As you can see “en” is the default language but if you reach a resource (for example a post) the path will be https://www.nicfab.it/en/en/posts/postname

@anvithks
Copy link
Owner

Problem: Embedding two PDFs in one page does not work

Solution: Use window.addEventListener("load",function(){ instead of window.onload = function(event) { and have unique IDs.

I made soma adaptions here: https://github.com/jakob-r/courseTheme/blob/main/layouts/shortcodes/pdfjs.html

If you are interested I can make a PR.

Apologies for the extremely reply but the last few months have been quite hectic.
If you still have the time and are willing to raise a PR it will be very helpful.
Cheers.

@nicfab
Copy link

nicfab commented Jan 10, 2022

I did it as you suggest, using window.addEventListener("load",function(){ instead of window.onload = function(event) {
and closing the round bracket at the end of the file just before the last line </script>
It doesn't work.
I also tried your file https://github.com/jakob-r/courseTheme/blob/main/layouts/shortcodes/pdfjs.html and it doesn't work.
Are you available to help me?

@FCACollin
Copy link

As a user, I confirm that I was very interested by hugo-embeded-pdf-shortcode, but as I was unable to display more than one pdf per page, I had to resign from using it, turning toward a more classic iframe. In other word, in my opinion this feature is a must.

@chtzvt
Copy link
Contributor

chtzvt commented Jun 3, 2022

Hello all,

I'm currently working on this, and have ended up with something mostly functional.

My modified version of the hugo-embed-pdf shortcode is available here: https://github.com/ctis-me/hugo-werc/blob/master/layouts/shortcodes/pdf.html

You can see a demonstration of multiple PDFs rendered in a page at the following link:

https://blog.ctis.me/2022/06/assorted-presentations/

Currently, I'm experiencing problems with transient corruption of text content when multiple PDFs are embedded. This behavior is inconsistent and occurs at random. I'm not entirely certain why this is, but would appreciate thoughts from others who have more experience working with the library.

@anvithks Once the text corruption issue is sorted, I can roll my changes back into the main project as a PR to enable this feature for everyone.

@chtzvt
Copy link
Contributor

chtzvt commented Jun 3, 2022

Actually, I just figured out the issue.

The PDF shortcode includes the pdf.js library in the page. If you have multiple of these, the library will be embedded multiple times. This appears to have been causing the unexpected behavior.

Thankfully, Hugo has a mechanism (.HasShortcode) to check for the existence of a shortcode which can be used in conditionals.

I updated my master header template to include the following. As a result, the corruption bug appears to have been fixed.

    {{ if .HasShortcode "pdf" }}
        <script type="text/javascript" src='{{"/pub/js/pdfjs/build/pdf.js" | relURL}}'></script>
    {{ end }}

@anvithks
Copy link
Owner

anvithks commented Jun 3, 2022

Thanks for the awesome work @ctrezevant !
Please do raise a PR when possible.
It would help sort out some of the issues.

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 a pull request may close this issue.

5 participants