Skip to content

fix(result): show wpm y-axis on both charts and open up share-card layout - #25

Merged
narze merged 1 commit into
mainfrom
fix/wpm-y-axis-and-share-card-layout
Jun 13, 2026
Merged

fix(result): show wpm y-axis on both charts and open up share-card layout#25
narze merged 1 commit into
mainfrom
fix/wpm-y-axis-and-share-card-layout

Conversation

@narze

@narze narze commented Jun 13, 2026

Copy link
Copy Markdown
Member

Problem

Two issues on the result screen, both about the WPM chart's missing y-axis:

  1. Live chart — the WpmChart markup already had a label="WPM" on the left axis, but it never painted. LayerChart wraps every tick/axis-title in a nested <svg class="overflow-visible"> and depends on that utility to override the SVG default overflow: hidden. This project's Tailwind doesn't generate overflow-visible (same gap the component's <style> already documents for fill-surface-content/stroke-surface-100), so each nested svg fell back to overflow: hidden and clipped the left axis — its tick labels are anchored end at x=0 (glyphs at negative x) and the rotated title sits at negative x, both outside their nested viewport. The bottom axis (positive/centered x) escaped, which is why the x-label showed but the y-label didn't.

  2. Share card (hand-drawn canvas, separate code) had no axis at all, and the overall layout felt packed with no whitespace.

Changes

WpmChart.svelte (live chart)

  • Pin overflow: visible on the chart's svg elements so the nested-svg clip no longer eats the y-axis.
  • Nudge the rotated title into the gutter via labelProps (it was pinned to the container's left edge); lowercase wpm to match the raw wpm / net wpm captions.

result-card.ts (share PNG)

  • Add a labelled wpm y-axis (rotated title, 0 / mid / max tick labels, gridlines) and a light seconds x-axis.
  • Round the axis up to a multiple of 20 so peaks keep headroom instead of clipping the top frame.
  • Open up the vertical rhythm (margins, section gaps, history line height) and lighten the hero number for more whitespace.
  • Reset textAlign/textBaseline after drawChart so the stats and footer no longer inherit the chart's centred/top text state.

Verification

  • Live chart: drove a real drill via Playwright, confirmed the wpm title + ticks now render (root-caused via DOM inspection — nested-svg overflow:hidden clip).
  • Share card: rendered the real PNG through renderResultBlob with synthetic 60s data and iterated on the screenshot.
  • result-card tests pass (2/2); svelte-check reports 0 errors / 0 warnings.

🤖 Generated with Claude Code

…yout

The live result chart had a y-axis label in the markup but it never
painted: LayerChart wraps each tick/title in a nested <svg> and relies on
a `.overflow-visible` utility that this project's Tailwind doesn't
generate, so the nested svg fell back to overflow:hidden and clipped the
left axis (ticks anchored `end` at x=0; rotated title at negative x).
Pin `overflow: visible` on the chart's svgs and nudge the title into the
gutter.

The share card (hand-drawn canvas) had no axis at all and felt cramped:
- Add a labelled wpm y-axis (rotated title, 0/mid/max ticks, gridlines)
  and a light seconds x-axis; round the axis up to a multiple of 20 so
  peaks keep headroom instead of clipping the top frame.
- Open up the vertical rhythm (margins, section gaps, history line
  height) and lighten the hero number for more whitespace.
- Reset textAlign/textBaseline after drawChart so the stats and footer
  no longer inherit the chart's centred/top text state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
learn-manoonchai Ready Ready Preview, Comment Jun 13, 2026 2:16pm

Request Review

@narze
narze merged commit 5d4e37b into main Jun 13, 2026
3 checks passed
@narze
narze deleted the fix/wpm-y-axis-and-share-card-layout branch June 13, 2026 15:05
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.

1 participant