Skip to content

fix: align link annotation hitboxes with rendered SVG elements - #372

Merged
HackbrettXXX merged 1 commit into
yWorks:masterfrom
amandeavor:fix/anchor-link-bounds
Aug 31, 2026
Merged

HackbrettXXX merged 1 commit into
yWorks:masterfrom
amandeavor:fix/anchor-link-bounds

Conversation

@amandeavor

@amandeavor amandeavor commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

SVG bounding boxes use a top-left origin, while PDF link annotations use a bottom-left origin. The previous conversion inverted the box's top edge without accounting for its height, placing the clickable area one link-height above the rendered element.

Solution

  • Convert the anchor's bottom edge when calculating the PDF annotation y-coordinate.
  • Add the reported rectangle case to the anchor fixture.
  • Assert the exact generated hitbox coordinates.
  • Refresh the PDF reference output.

Verification

  • npm run build passed
  • ESLint passed for the changed implementation and unit test
  • Focused Karma run in Chrome Headless passed for anchor
  • Generated hitbox matches [180, 190, 100, 100]

Fixes #363

@amandeavor amandeavor changed the title fix: align SVG link annotation bounds fix: align link annotation hitboxes with rendered SVG elements Aug 2, 2026
@amandeavor
amandeavor marked this pull request as ready for review August 2, 2026 07:18
Copilot AI review requested due to automatic review settings August 2, 2026 07:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an origin mismatch between SVG bounding boxes (top-left origin) and PDF link annotations (bottom-left origin) so that anchor link hitboxes align with the rendered SVG elements.

Changes:

  • Adjust link-annotation Y coordinate calculation to use the anchor bounding box’s bottom edge.
  • Extend the anchor fixture SVG with a concrete rectangle case that previously reproduced the offset issue.
  • Add a unit assertion that inspects jsPDF#link calls and validates the exact generated hitbox coordinates.

Reviewed changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/nodes/anchor.ts Fixes link hitbox Y-coordinate conversion by anchoring the annotation to the bbox bottom edge.
test/specs/anchor/spec.svg Adds a deterministic rectangle-in-anchor case to validate hitbox placement visually and programmatically.
test/unit/all.spec.js Hooks pdf.link during the anchor test to assert the generated hitbox coordinates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/unit/all.spec.js Outdated
Comment on lines +46 to +49
if (name === 'anchor') {
const hitbox = linkCalls.find(args => args[4].url === 'https://example.com/hitbox')
expect(hitbox.slice(0, 4)).to.deep.equal([180, 190, 100, 100])
}

@HackbrettXXX HackbrettXXX left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for this PR. It correctly fixes the bounding box of the rect link. However the text bounding boxes are incorrect now.

Comment thread test/unit/all.spec.js Outdated
Comment on lines +26 to +34
const linkCalls = []

if (name === 'anchor') {
const link = pdf.link
pdf.link = (...args) => {
linkCalls.push(args)
return link.apply(pdf, args)
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we need this. The reference file already ensures that the link hitbox is correct and I don't like having special cases like this in the spec file.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On the main branch I've switched to vitest, so individual unit test can now be realized much easier.

@amandeavor
amandeavor force-pushed the fix/anchor-link-bounds branch from 3e38d09 to e1532e5 Compare August 28, 2026 11:55
@amandeavor

Copy link
Copy Markdown
Contributor Author

Updated based on the review feedback and the Vitest migration:\n\n- rebased onto current master / v2.8.0\n- moved the rectangular-link regression into an individual Vitest case\n- normalized text bounding boxes to top-left coordinates so the rectangle fix does not shift existing text links\n- added explicit regression coverage for both rectangle and text hitboxes\n\nVerified locally with the two focused browser tests, TypeScript type-checking, oxlint, and the production build. The existing PDF snapshot test remains unchanged.

@HackbrettXXX HackbrettXXX left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good now, thanks!

@HackbrettXXX
HackbrettXXX merged commit 70a92d8 into yWorks:master Aug 31, 2026
2 checks passed
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.

Hit box of links from anchor elements is wrong

3 participants