Skip to content

Implement internal linking and two pass single page rendering for generated PDFs#13161

Open
benbowler wants to merge 2 commits into
developfrom
enhancement/12553-pdf-internal-linking-two-pass-rendering
Open

Implement internal linking and two pass single page rendering for generated PDFs#13161
benbowler wants to merge 2 commits into
developfrom
enhancement/12553-pdf-internal-linking-two-pass-rendering

Conversation

@benbowler

@benbowler benbowler commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Addresses issue:

Relevant technical choices

  • The BUILDING stage now renders the report twice: a discarded measurement pass on an overheight page captures the content height and the section positions via the Document onRender callback, then the final pass renders the page bounded to the measured height plus a fixed bottom padding.
  • measurePDFContentHeight reads the undocumented _INTERNAL__LAYOUT__DATA_ layout result and throws a clear error naming the expected property path when the shape is unrecognised, so a library upgrade fails the export cleanly instead of producing a broken PDF. Errors thrown inside onRender are captured and rethrown after the render, since @react-pdf may swallow exceptions raised in its render pipeline.
  • Header section chips are wrapped in <Link src="#section-{slug}">. @react-pdf registers a named destination from a node's parent-relative top, so an id nested inside the report body lands the viewer a header height above the section. To anchor correctly, the measurement pass also extracts each section's absolute top (extractPDFSectionAnchors), and the final pass pins zero-size, absolutely positioned anchor views carrying the ids directly on the page, where relative and absolute tops coincide. Verified against the real library: destinations land exactly at each section's position.
  • @react-pdf sizes a wrap={false} page to its content and ignores the requested height, so the fixed bottom padding is applied through the page's paddingBottom; this makes the rendered page height equal the computed measured height plus padding, with the explicit size height kept as an upper bound.
  • The Jest mock's toBlob() now fires the document's onRender callback with a layout fixture mirroring the internal shape, so the two-pass flow is exercised in unit tests.

PR Author Checklist

  • My code is tested and passes existing unit tests.
  • My code has an appropriate set of unit tests which all pass.
  • My code is backward-compatible with WordPress 5.2 and PHP 7.4.
  • My code follows the WordPress coding standards.
  • My code has proper inline documentation.
  • I have added a QA Brief on the issue linked above.
  • I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

  • Run the code.
  • Ensure the acceptance criteria are satisfied.
  • Reassess the implementation with the IB.
  • Ensure no unrelated changes are included.
  • Ensure CI checks pass.
  • Check Storybook where applicable.
  • Ensure there is a QA Brief.
  • Ensure there are no unexpected significant changes to file sizes.

Merge Reviewer Checklist

  • Ensure the PR has the correct target branch.
  • Double-check that the PR is okay to be merged.
  • Ensure the corresponding issue has a ZenHub release assigned.
  • Add a changelog message to the issue.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 This comment is automatically updated by CI workflows. Each section is managed independently.

📚 Storybook for f9b181e:

📦 Build files for f9b181e:

🎭 Playwright reports for f9b181e:

@zutigrm zutigrm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @benbowler looks good overall, I just have one question bellow

Comment on lines +607 to +622
await pdf(
<DashboardReport
siteName={ reportSiteName }
siteURL={ referenceSiteURL || '' }
dashboardURL={ dashboardURL || '' }
dateRange={ {
startDate: dates.startDate,
endDate: dates.endDate,
{ ...reportProps }
pageHeight={ PDF_MEASURE_PAGE_HEIGHT }
onRender={ ( layout ) => {
try {
measuredHeight =
measurePDFContentHeight( layout );
sectionAnchors =
extractPDFSectionAnchors( layout );
} catch ( error ) {
measureError = error;
}
} }
sections={ sections }
helpCenterURL="https://sitekit.withgoogle.com/support/?doc=get-support"
privacyPolicyURL="https://policies.google.com/privacy"
areas={ areas }
emailReportingSetupURL={ emailReportingSetupURL }
/>
).toBlob();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Before we had run blob render once (bellow), now we have 2 runs, and the timout for build is still armed using old 15s timeout

armStageTimeout( BUILDING_TIMEOUT_MS );

Is this still capable of running two blob renders under 15s, or should we reconsider increasing the timeout? In case running 2 of them add a bit of time, we might run into scenario where almost finished reort aborts due to the timeout etc. WDYT?

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