Skip to content

RS-22108: Wait for fonts before laying out the heatmap - #60

Open
JustinCCYap wants to merge 10 commits into
masterfrom
RS-22108
Open

RS-22108: Wait for fonts before laying out the heatmap#60
JustinCCYap wants to merge 10 commits into
masterfrom
RS-22108

Conversation

@JustinCCYap

@JustinCCYap JustinCCYap commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Fixes RS-22108, and its duplicate RS-23543: heatmap axis labels are shifted and truncated in a PPT or PDF export when the document uses a custom font, intermittently and not reproducibly.

Diagnosis

The chart sizes its axis bands by measuring throwaway <text> elements in the live DOM (rhtmlLabelUtils getSingleLineLabelDimensions, called from theSrc/scripts/lib/components/parts/labelUtilsWrapper.js:52, consumed at yAxis.js:34-38 and xAxis.js:59-75), and it never re-measures.

In the Displayr export page the custom fonts arrive through an asynchronous CSS @import, so the chart can lay out with fallback font metrics. The widget then reported itself ready synchronously, and rhtmlwidget-status is the only readiness signal the export screenshot waits on. The custom font swaps in after layout, leaving the labels in bands sized for the wrong font, so they shift, truncate with an ellipsis, or are clipped at the SVG edge.

On screen the problem is usually invisible because any resize triggers a full re-render and re-measure (rhtmlHeatmap.factory.js:29-32). An export renders once, which is why only the export is affected, and why identical steps give different results: it is a race against the font load.

Solution

The chart now waits for the fonts it draws with before it measures anything.

  1. New theSrc/scripts/lib/fonts.js. fontFamiliesInUse(options) returns the distinct values of every *_font_family option, so no hardcoded list of components is needed. waitForFonts(options) calls document.fonts.load() for each family in normal and bold, then awaits document.fonts.ready, bounded by a 3s timeout so an unloadable font can only delay the chart, never prevent it. The explicit load() matters: fonts.ready alone can resolve before a face that nothing has rendered yet is fetched.
  2. heatmapOuter.js gates rendering on Promise.all([loadImage(image), waitForFonts(options)]), so fonts are awaited alongside the image data and cost no extra time when already available.
  3. heatmapOuter.js claims rhtmlwidget-status as loading before the asynchronous work starts. It was previously only claimed inside the Heatmap constructor, which runs after the image load, so during the wait an export would see a widget that is not loading and could screenshot an empty SVG.
  4. heatmapOuter.js reports ready on the error path too, so a failed render cannot leave an export waiting out its screenshot timeout.

Two hardening changes came out of reviewing the above, both of which matter only because step 3 widened the window between claiming loading and reporting ready:

  1. A fonts.load() call is guarded against a synchronous throw. waitForFonts is evaluated as an argument to Promise.all, so a throw would escape before the chain has a catch and leave the status at loading; Blink throws rather than rejects when it cannot parse a font shorthand.
  2. Both status writes check that this render's svg is still in the container. A resize re-renders without cancelling the render it interrupts, so an older chain could otherwise settle and mark a newer, unfinished chart ready.

Included refactor

heatmapcore no longer writes rhtmlwidget-status on its container — it was reaching outside itself to set an attribute on its parent, which left the claim in one file and the release in another. All three transitions now sit in heatmapOuter.js, the code that owns the render. Behaviour and timing are unchanged, and heatmapOuter.js is the only caller of heatmapcore anywhere in the Displayr org.

Tests

theSrc/scripts/lib/fonts.jest.test.js covers the option collection and the failure paths that must not stop a chart rendering: an unloadable font, a synchronous throw from load, a font set with no load method, no document.fonts at all, and a font set that never becomes ready. gulp testSpecs is 11 green.

Not unit covered, and worth a reviewer's eye: the status lifecycle and the stale-render guard in heatmapOuter.js, which needs a DOM, Image and canvas to exercise and has no harness in this repo. The visual regression suite waits on div[rhtmlwidget-status=ready], so a broken ready transition would show up there as timeouts.

Warning

Author: before requesting review, attach evidence the bug is fixed.
A screenshot, screen recording, test log, or step-by-step repro showing the
original failure no longer happens. A reviewer cannot tell whether this PR
actually fixes the ticket from the diff alone - make it easy for them.

🤖 Generated with Claude Code

JustinCCYap and others added 10 commits August 20, 2026 17:01
Labels are sized by measuring them in the DOM, so a font that arrives after
layout leaves them positioned and truncated for the fallback font's metrics.
On screen any resize re-renders and hides this, but an image export renders
once, so the exported chart does not match what Displayr shows.

Load every configured font family and wait for the document's fonts, bounded
by a timeout, before rendering. The widget status is now set to loading before
that wait, so an export cannot screenshot the chart while it is still waiting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jsdom does not allow document to be reassigned, so the stubbed font set was
being ignored and waitForFonts took its no font set path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Quote the family in the font shorthand so it is always parseable, which
removes the need to guard against a synchronous throw, flatten the two
nested loops into one, and drop the timeout constant from the exports
since nothing imports it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
heatmapcore reached outside itself to set rhtmlwidget-status on its
container, which left the loading claim in heatmapOuter and its release in
heatmapcore. Move the ready write to heatmapOuter, so the whole lifecycle
sits with the code that owns the render.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
waitForFonts is evaluated as an argument to Promise.all, so a synchronous
throw escapes before the render chain has a catch, and the status stays at
loading until the export times out. Blink throws rather than rejects when it
cannot parse the font shorthand, and quoting the family does not rule that
out for a family name containing a quote or a trailing backslash, so restore
the guard removed in 9efdba3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A resize renders from scratch without cancelling the render it interrupts,
so an older chain can settle after its svg has been discarded. Both status
writes now check that this render's svg is still in the container, so a
stale chain cannot mark a newer, unfinished chart as ready.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JustinCCYap
JustinCCYap marked this pull request as ready for review August 21, 2026 07:23
@JustinCCYap
JustinCCYap requested a review from chschan August 21, 2026 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant