fix: make prop-derived chart values reactive (Area label position, Sparkline series type) - #3311
fix: make prop-derived chart values reactive (Area label position, Sparkline series type)#3311rajanpanth wants to merge 2 commits into
Conversation
Area computed its default label position from swapXY and Sparkline its series type from type with plain let declarations, so both froze at mount-time values while their consumers are reactive: toggling swapXY left area labels falling back to 'top' instead of 'right', and changing a sparkline's type from bar to line kept rendering bars. Same class as the BarChart stacked100 fix (evidence-dev#3310): derive with $: so the reactive consumers see current values.
Untruncated re-sweep of the viz tree found the identical pattern in Line.svelte (the earlier sweep output was truncated). _Chart's ySet/xSet are intentionally non-reactive per their in-code comment and stay as-is.
|
Pushed a follow-up commit: an untruncated re-sweep of the viz tree found the identical pattern in |
|
Friendly follow-up on this PR 👋 When you have a moment, could you please take a look? I’m happy to rebase, update tests, or make any requested changes. Thanks! |
|
The external-contributor test workflow is still waiting for maintainer approval. Could a maintainer approve the workflow run when convenient? The deploy previews and visible checks are otherwise complete. |
Sibling fix to #3310 — same bug class, found by sweeping the other viz components for it.
Problem
Two chart components derive values from props with plain
letdeclarations, freezing them at mount-time while their consumers are reactive:Area.svelte:let defaultLabelPosition = swapXY ? 'right' : 'top'— togglingswapXYafter mount leaves the reactivelabelPositionfallback stale (labels fall back totopon a swapped chart instead ofright)._Sparkline.svelte:let seriesType = type === 'area' ? 'line' : type—seriesTypeis consumed inside the$: tryconfig block, so mounting asbarand switching tolinekeeps rendering bars.Fix
Derive both with
$:so the reactive consumers see current values — the same one-line pattern as thestacked100fix in #3310. Changeset included (patch,@evidence-dev/core-components).Deliberately excluded from the sweep: the
isInitial/queryIDmount-time captures incore/*(intentionally non-reactive — they record initial state), and function-locallets.Note on tests: #3310 establishes the regression-spec pattern for this class on BarChart; these two are the same one-line derivation change. Happy to add per-component specs if you want them — Area mounts inside chart context, so I kept this PR surgical.