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

PdfOutline (Outline/Bookmarking functionality) #486

Closed
wants to merge 313 commits into from

Conversation

aready87
Copy link

Hi @Hopding,

We put together PDFOutline that adds the outlines (aka bookmarks, table of contents) functionality as specified in Adobe's PDF Specification Section 12.3.3 /Outlines. We adhered to the PDF Specification and also how PDFLib was organized and structured as closely as possible (It is structured very closely to PDFPage that wraps PDFPageLeaf; PDFOutline wraps PDFOutlines).

Scratchpad's index.ts pretty much captures the functionalities that we've added. Notably:

  • PDFDocument's CreateOptions now has useOutlines boolean which adds the PageMode UseOutlines for displaying the outline panel when the document is opened (Table 28, pg 74)
  • PDFDocument's addOutline and insertOutline adds a top-level outline (PDFOutline) to the document, with option: expanded (for expanded display), linkPage (link the outline to a given PDFPage) or linkIndex (link the outline to a given index). This returns a PDFOutline.
  • PDFOutline (once received from the above point) also has a similar addOutline and insertOutline that adds a nested outline to said PDFOutline with the same options (expanded, linkPage and linkIndex.
  • PDFOutline also has the following:
    i) setTitle to change the label/title of the outline
    ii) linkPage to add/change the outline's link to a given PDFPage
    iii) linkIndex to add/change the outline's link to a given index
    iv) setExpanded to change the expanded boolean
    v) remove to delete the outline and its children

We also tried to provide as much comments where necessary, following the style and flow of the other comments in the code (including a shoutout to another library).

This will resolve #450 and is a more complete solution to #127, and hopefully makes it easier to implement a feature request #218.

This passes scratchpad:start without issue and passes yarn typecheck, yarn test and successfully builds with yarn build. But it fails the integration tests. Could you please help us in implementing PDFOutline and resolving the errors? There's likely more to discuss and more to complete (e.g. writing tests and documentation) but we'll give you an opportunity to review the code first and approve the overall structure. Feel free to reach out to us.

Thank you.

-Real @closin-inc

@aready87
Copy link
Author

@Hopding: Hi Hopding, hope you've been well. Apologies for the delayed response; We've been quite busy with beta testing our core product. I went ahead and applied the two requested changes: 1) to the options { page: PDFPage} and 2) including PDFDocument.setDisplayMode(DisplayMode).

I am however having difficulty with the suggested compositional pattern without breaking the root outlines' functionality. I just spun my wheel for the last few hours without much success. Sadly I won't be able to get back to this until August (possibly even later); so if you or anyone else want to take a stab at the compositional pattern, I would actually be grateful. Apologies for leaving it unfinished for the time being.

@Hopding
Copy link
Owner

Hopding commented Jul 5, 2020

@aready87 No worries! Thanks for working on this! (especially since you've been busy with testing your product)

I'm deep in the weeds implementing a form creation/filling API for pdf-lib, so I'm afraid I don't have time to pick this up right now. If you're able to explain what difficulties you ran into I'd be happy to help you (or anybody else) figure out a solution though.

if (!parent) {
dict.set(PDFName.Type, PDFName.Outlines);
} else {
dict.set(PDFName.Title, title ? PDFString.of(title) : title);

Choose a reason for hiding this comment

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

Copy link

@divinehawk divinehawk Jul 25, 2020

Choose a reason for hiding this comment

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

Setting the outline title with a string with an unmatched parentheses causes a corrupt PDF.
Per #516 , using PDFHexString appears to fix this problem.

divinehawk@57ace82

@drom
Copy link

drom commented Aug 2, 2020

A very important feature to have! Any plans merging it?

@Hopding
Copy link
Owner

Hopding commented Aug 3, 2020

@drom Agreed! I'd very much like to get this included in the library. All that remains to do so is the completion of the compositional pattern explained above (#486 (comment)). I do not personally have time to work on this yet. If you're interested in helping out, that would definitely speed things along.

@drom
Copy link

drom commented Aug 20, 2020

@Hopding I would love to help, but after multiple attempts, my brain was not able to comprehend the sheer design complexity of this library. So as a pacient user I would just wait for a resolution.

@jainshubham97
Copy link

Hi @Hopding @aready87 , Thanks for having this wonderful feature in the pipeline. However, I need to add hyperlinks to pages in the PDF and I believe this PR has a sub-component (linkPages) that can help me out. Is there any way I can extract it and add it to my library manually? This feature would take time and I am not capable enough to work on the compositional patterns. Appreciate the help.

@Hopding
Copy link
Owner

Hopding commented Sep 21, 2020

@jainshubham97 I’d suggest taking a look at #123 (comment). It has a working example showing how to create links to pages in a document

@jainshubham97
Copy link

@Hopding Thanks brother! Worked like a charm. It saved me many days of effort. I will try to add a high-level API for this if I could as mentioned in #555 (Feature)

@Hopding
Copy link
Owner

Hopding commented Sep 22, 2020

@jainshubham97 Glad it worked for you! I look forward to your PR. Let me know if you have any questions.

@stargazing-dino
Copy link

I know priorities change but is there any hope of getting this across the finish line? Seems very close

@Hopding
Copy link
Owner

Hopding commented Aug 31, 2021

@Nolence it is indeed very close and I'd love to see it move across the finish line. I was hoping to do the work myself, but I've just got to much on my plate at the moment. Happy to answer questions if anybody is interested in finishing it up though!

If anybody does decide to pick this up, please note that you'll need to either update this PR (via a force push) or open a new PR with your changes. The tl;dr is that I recently had to do a fairly extensive git history rewrite that caused a bunch of conflicts. Please read https://github.com/Hopding/pdf-lib#git-history-rewrite for details. Apologies for the hassle!

@Hopding
Copy link
Owner

Hopding commented Sep 20, 2021

Closing this for now. Added outline support to the roadmap in #998.

@Hopding Hopding closed this Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] API to create an outline (table of contents) for PDF readers