Set tab order when document is tagged by acrollet · Pull Request #1449 · foliojs/pdfkit

acrollet · 2023-05-23T23:55:18Z

Changes

relates to department-of-veterans-affairs/va.gov-team#58587

This PR sets tab order to "Structure" when a document is tagged.

Unit Tests
Documentation - N/A
Update CHANGELOG.md
Ready to be merged

insightfuls

@acrollet , firstly, let me say thanks for putting this together. Despite being simple, I hadn't managed to look at it, and having something concrete to look at is really helpful. This is certainly put together well, and it's good to see that it's tested. For a couple of reasons I'll explain inline, I think a slightly different approach would be preferable, though. I think it should be easy to adjust, if you can spare a little more time for this.

lib/page.js

insightfuls

Thanks @acrollet , overall, this looks great. There's nothing wrong with it, so I'm happy to give an approving review. I've left a comment inline, however, suggesting simplifying a condition, which also makes it more consistent with nearby code.

lib/mixins/markings.js

lib/page.js

acrollet

Adding suggestion

lib/mixins/markings.js

insightfuls · 2023-05-26T01:46:06Z

@blikblum , I'm happy for this to be merged if you are, or would you like @liborm85 or @devongovett to give it a review as well?

acrollet · 2023-05-26T03:08:15Z

acrollet@MacBook-Air pdfkit % npm run test                            

> pdfkit@0.13.0 test
> jest -i

 PASS  tests/visual/pdfmake/qrcode.spec.js
 PASS  tests/visual/pdfmake/tables.spec.js
 PASS  tests/visual/pdfmake/columns_simple.spec.js
 PASS  tests/visual/pdfmake/lists.spec.js
 PASS  tests/visual/pdfmake/page_references.spec.js
 PASS  tests/visual/pdfmake/toc.spec.js
 PASS  tests/visual/pdfmake/background.spec.js
 PASS  tests/visual/pdfmake/text_decorations.spec.js
 PASS  tests/visual/pdfmake/watermark.spec.js
 PASS  tests/visual/pdfmake/absolute.spec.js
 PASS  tests/visual/pdfmake/images.spec.js
 PASS  tests/unit/png.spec.js
 PASS  tests/unit/acroform.spec.js
 PASS  tests/visual/pdfmake/basics.spec.js
 PASS  tests/visual/vector.spec.js
 PASS  tests/unit/vector.spec.js
 PASS  tests/unit/annotations.spec.js
 PASS  tests/unit/attachments.spec.js
 PASS  tests/unit/text.spec.js
 PASS  tests/unit/virtual-fs.spec.js
 PASS  tests/unit/pdfa3.spec.js
 PASS  tests/unit/pdfa2.spec.js
 PASS  tests/unit/pdfa1.spec.js
 PASS  tests/unit/pattern.spec.js
 PASS  tests/unit/saslprep.spec.js
 PASS  tests/unit/trailer.spec.js
 PASS  tests/unit/reference.spec.js
 PASS  tests/unit/document.spec.js
 PASS  tests/visual/fonts.spec.js
 PASS  tests/unit/font.spec.js
 PASS  tests/unit/gradient.spec.js
 PASS  tests/visual/text.spec.js
 PASS  tests/unit/color.spec.js
 PASS  tests/unit/metadata.spec.js
 PASS  tests/unit/object.spec.js
 PASS  tests/unit/markings.spec.js

Test Suites: 36 passed, 36 total
Tests:       2 skipped, 142 passed, 144 total
Snapshots:   52 passed, 52 total
Time:        40.663 s
Ran all test suites.

acrollet · 2023-06-15T14:23:01Z

Hi folks, just wondering if there's anything else needed on this PR. Thanks! 🙂

acrollet · 2023-09-13T19:36:38Z

@blikblum @insightfuls just wondering if there's anything else needed or if this could be merged? Thanks!

CarlBohman · 2024-07-23T04:47:43Z

I'm using PDFKit with TypeScript. For a single-page document (which is what I am working with right now), the suggested fix works for me as follows:

const doc = new PDFDocument(...);
// @ts-ignore
doc.page.dictionary.data.Tabs = 'S';

Adding Tabs to the type for the data property of the PDFKitReference class (defined in @types/pdfkit) removes the need for @ts-ignore.

Any idea when this pull request will be merged and made part of a general release?

JAshique · 2024-10-25T11:50:05Z

Hi @CarlBohman, thanks for your suggestion. It seems like your suggest approach works for single page PDFs. I was wondering if there is a way to do it for multiple pages. I'm thinking of the following approach.

let doc = new PDFDocument({
    tagged: true,
    lang: 'en-CA',
    pdfVersion: '1.7',
    info: {
    Title: 'Big Summary',
    Author: 'JAshique',
  },
  displayTitle: true,
});
    
doc.page.dictionary.data.Tabs = 'S';
let docStream = doc.pipe(blobStream());

docStream.on('pageAdded', () => { // Executes when a doc.addPage() is called 
    doc.page.dictionary.data.Tabs = 'S';
});

I have scenarios where I'll be generating multiple pages. Regardless, I will check it myself to see if this makes a difference. Ideally, I'd like this PR to be merged so this can be handled by the library itself.

Thanks :)

JAshique · 2024-10-28T13:11:13Z

UPDATE: The above mentioned snippet works for multiple pages.

if (document.y > 600) {
    document.addPage();
    document.page.dictionary.data.Tabs = 'S';
}

liborm85 · 2024-12-26T13:35:26Z

Merged. Thanks.

(foliojs#1449) * Set tab order when document is tagged * Update approach and add inverse test * Revert page dictionary setup * Update lib/mixins/markings.js * Update kitchen-sink-accessible.pdf * Update CHANGELOG.md --------- Co-authored-by: Libor M. <liborm85@gmail.com>

Set tab order when document is tagged

dcffcda

acrollet mentioned this pull request May 23, 2023

PDF accessibility check #1260

Closed

insightfuls suggested changes May 24, 2023

View reviewed changes

lib/page.js Outdated Show resolved Hide resolved

acrollet added 2 commits May 25, 2023 15:30

Update approach and add inverse test

1c9ce2c

Revert page dictionary setup

3b82e0e

insightfuls approved these changes May 26, 2023

View reviewed changes

lib/mixins/markings.js Outdated Show resolved Hide resolved

lib/page.js Show resolved Hide resolved

acrollet commented May 26, 2023

View reviewed changes

lib/mixins/markings.js Outdated Show resolved Hide resolved

Update lib/mixins/markings.js

31b505e

insightfuls approved these changes May 26, 2023

View reviewed changes

acrollet mentioned this pull request Jun 2, 2023

Review exported PDFs from "MHV on VA.gov" team department-of-veterans-affairs/va.gov-team#58587

Closed

12 tasks

acrollet mentioned this pull request Oct 6, 2023

Review exported PDFs from "MHV Rx" team department-of-veterans-affairs/va.gov-team#66799

Closed

12 tasks

liborm85 added 3 commits December 26, 2024 14:25

Merge branch 'master' into pr/1449

9a5fe90

Update kitchen-sink-accessible.pdf

57b24a9

Update CHANGELOG.md

d7b3e9b

liborm85 merged commit 8b20d07 into foliojs:master Dec 26, 2024

Conversation

acrollet commented May 23, 2023

Changes

Uh oh!

insightfuls left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

insightfuls left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

acrollet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

insightfuls commented May 26, 2023

Uh oh!

acrollet commented May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acrollet commented Jun 15, 2023

Uh oh!

acrollet commented Sep 13, 2023

Uh oh!

CarlBohman commented Jul 23, 2024

Uh oh!

JAshique commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JAshique commented Oct 28, 2024

Uh oh!

liborm85 commented Dec 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

acrollet commented May 26, 2023 •

edited

Loading

JAshique commented Oct 25, 2024 •

edited

Loading