fix(result): show wpm y-axis on both charts and open up share-card layout - #25
Merged
Merged
Conversation
…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>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two issues on the result screen, both about the WPM chart's missing y-axis:
Live chart — the
WpmChartmarkup already had alabel="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 defaultoverflow: hidden. This project's Tailwind doesn't generateoverflow-visible(same gap the component's<style>already documents forfill-surface-content/stroke-surface-100), so each nested svg fell back tooverflow: hiddenand clipped the left axis — its tick labels are anchoredendat 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.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)overflow: visibleon the chart's svg elements so the nested-svg clip no longer eats the y-axis.labelProps(it was pinned to the container's left edge); lowercasewpmto match theraw wpm/net wpmcaptions.result-card.ts(share PNG)0 / mid / maxtick labels, gridlines) and a light seconds x-axis.textAlign/textBaselineafterdrawChartso the stats and footer no longer inherit the chart's centred/top text state.Verification
wpmtitle + ticks now render (root-caused via DOM inspection — nested-svgoverflow:hiddenclip).renderResultBlobwith synthetic 60s data and iterated on the screenshot.result-cardtests pass (2/2);svelte-checkreports 0 errors / 0 warnings.🤖 Generated with Claude Code