Fix: Hyperlinks in PDF not showing correctly when printing the radar#405
Fix: Hyperlinks in PDF not showing correctly when printing the radar#405bob10042 wants to merge 1 commit intothoughtworks:masterfrom
Conversation
When printing the radar to PDF, links in blip descriptions that use
relative URLs (e.g., '/radar/Techniques/continuous-delivery') were
displaying as just the path rather than the full clickable URL.
Changes:
- Add convertRelativeUrlsToAbsolute() utility function in urlUtils.js
- Apply URL conversion when rendering blip descriptions in:
- quadrantTables.js (main quadrant view)
- radar.js (radar view descriptions)
- Add comprehensive unit tests for the new function
The CSS print styles show hrefs after link text using:
a:after { content: ' <' attr(href) '> '; }
This fix ensures those hrefs are complete URLs like:
'https://radar.thoughtworks.com/radar/Techniques/continuous-delivery'
instead of just:
'/radar/Techniques/continuous-delivery'
Fixes thoughtworks#399
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #399 where hyperlinks in blip descriptions display incorrectly when printing the radar to PDF. The problem occurs because CSS print styles (in _pdfPage.scss line 117) display href attributes after link text, and relative URLs were showing as just paths instead of complete URLs.
Changes:
- Added
convertRelativeUrlsToAbsolute()utility function to convert relative URLs to absolute URLs in HTML content - Applied URL conversion to blip descriptions in both
quadrantTables.jsandradar.js - Added 11 comprehensive unit tests covering various URL formats and edge cases
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/util/urlUtils.js | Added new utility function convertRelativeUrlsToAbsolute() to convert relative URLs (root-relative, relative, and protocol-relative) to absolute URLs using regex-based replacement |
| src/graphing/radar.js | Applied URL conversion to blip descriptions when rendering them in the radar visualization |
| src/graphing/components/quadrantTables.js | Applied URL conversion to blip descriptions when rendering them in quadrant tables |
| spec/util/urlUtils-spec.js | Added 11 unit tests covering empty/null/undefined inputs, absolute URLs, mailto/tel links, root-relative URLs, relative URLs, protocol-relative URLs, multiple links, and single quotes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const expected = '<a href="https://radar.thoughtworks.com/radar/page">Link</a>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(expected) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Consider adding test cases for URLs with query parameters and fragments to ensure they are handled correctly. For example:
<a href="/path?query=value">Link</a><a href="/path#section">Link</a><a href="/path?query=value&other=param#section">Link</a>
These are common URL patterns that should be verified to work correctly with the regex.
| it('should not modify absolute URLs', () => { | ||
| const html = '<a href="https://example.com/page">Link</a>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(html) | ||
| }) |
There was a problem hiding this comment.
Consider adding a test case for http:// URLs (without the 's') to ensure the regex properly handles both HTTP and HTTPS protocols. While the regex pattern https? correctly handles both, an explicit test would make this behavior clearer and prevent regressions.
|
|
||
| // Convert relative href attributes to absolute URLs | ||
| return html.replace( | ||
| /href=["'](?!https?:\/\/|mailto:|tel:)([^"']+)["']/gi, |
There was a problem hiding this comment.
As a defense-in-depth measure, consider adding other URL schemes to the negative lookahead pattern: (?!https?:\/\/|mailto:|tel:|javascript:|data:). While the sanitize-html library already filters dangerous schemes like javascript: and data:, explicitly excluding them in the URL conversion function provides an additional layer of protection and makes the security intent more explicit in the code.
| /href=["'](?!https?:\/\/|mailto:|tel:)([^"']+)["']/gi, | |
| /href=["'](?!https?:\/\/|mailto:|tel:|javascript:|data:)([^"']+)["']/gi, |
Summary
Fixes the issue where hyperlinks in blip descriptions display incorrectly when using Print the radar to generate a PDF.
Problem
When printing, links in descriptions that use relative URLs were showing as just the path instead of the full URL.
Changes
Technical Details
The CSS print styles display hrefs after link text. This fix ensures the href attributes contain complete URLs instead of relative paths.
Testing
Fixes #399