Storybook polish: interactive theming controls for widgets (#64)#73
Storybook polish: interactive theming controls for widgets (#64)#73Godbrand0 wants to merge 7 commits into
Conversation
…tories Adds interactive Storybook controls for defaultTheme and themeOverrides across all four widget showcase stories (StreamingWidget, CitizenClaimWidget, StakingMigrationWidget, ClaimWidget) plus the Card/GlowCard/Stepper/Drawer design-system primitives, so integrators can experiment with theming live in the Controls panel instead of editing story code. Extracts the cobalt/teal brand override presets into a shared helper reused across widgets.
CitizenClaimWidget.mdx and StakingMigrationWidget.mdx were missing the "How to mount it" code snippet that ClaimWidget.mdx already has, so integrators had no copy-paste starting point for those two widgets. Adds the same Source block pattern to both, and adds a one-line note to all three widget docs pages (CitizenClaimWidget, StakingMigrationWidget, StreamingWidget) pointing at the new defaultTheme/themeOverrides Storybook controls added in this branch.
Found during manual visual verification of issue GoodDollar#64 (each control must visibly change what renders): the Stepper Controllable story passed a static STEPS array with hardcoded per-step `status` values, so moving the `activeStepId` control only auto-scrolled the list — the blue "active" highlight stayed pinned to whichever step had status:'active' in the array (always "Submit migration"), regardless of the selected control value. Now recomputes each step's status relative to the chosen activeStepId so the control drives a real state change.
Found during manual verification: StreamingWidget's card surfaces use their own named Tamagui theme keys (StreamRow, PoolRow, BalanceCard, EmptyStateCard, ErrorStateCard, SetStreamFormCard — see packages/streaming-widget/src/components/shared.tsx), none of which overlap with the Claim-family keys (ClaimCard, ClaimActionGlow, etc.) the shared cobalt/teal presets originally targeted. So toggling brandPreset on the StreamingWidget showcase story changed nothing. Adds matching border/shadow overrides for StreamingWidget's card names to both presets so the control has a real effect there too. StakingMigrationWidget already reuses the ClaimCard/StreakCard names (packages/staking-migration-widget/src/migrationWidgetComponents.ts), so it was already covered — it just isn't visible on the "no wallet" fallback screen, since that screen renders plain text with no themed Card at all.
The Controls panel is auto-generated for every story of a component, so it appears (and looks interactive) on Default/WithAction-style stories too — but their render functions use fixed hardcoded props so those stories stay stable for other docs pages and Playwright screenshot tests. Only the "Controllable" story actually reads args. This was confusing during manual verification, so each Default story across Card/GlowCard/Stepper/Drawer now says so explicitly.
… fallback Found during manual verification: InjectedWalletStory threaded themeOverrides through to StreamingWidgetStoryShell (the "wallet connected" path) but not to PreviewStoryShell (the "no wallet" fallback path, which most reviewers hit since they don't have a matching injected wallet). So brandPreset silently did nothing on the common case of that story. PreviewStoryShell and its underlying StreamingWidgetPreview call now also receive themeOverrides.
…re stories Per issue GoodDollar#64's literal DoD ("all stories are accompanied with controls"), the QA runtime-fixture stories previously had zero controls. Threads defaultTheme/themeOverrides through every QA helper function across all three widgets (20 StreamingWidget states, 1 CitizenClaimWidget state, 8 StakingMigrationWidget states) and wires matching argTypes into each QA story file, following the same pattern already used on the Showcase stories. QA determinism for Playwright is unaffected: automated tests navigate directly to story IDs without touching the Controls panel, so the fixed default args (dark theme, no brand preset) still apply during test runs. A human reviewer gets the same live override on top when browsing QA stories manually. LightThemePopulated/LightThemeReady keep their hardcoded defaultTheme="light" mount (that's the point of those specific stories) but now also accept the brandPreset control on top.
L03TJ3
requested review from
a team and
0xOlivanode
and removed request for
a team
0xOlivanode
left a comment
0xOlivanode
left a comment
There was a problem hiding this comment.
Code itself is solid, the Stepper activeStepId fix is a real and correctly implemented fix, the cobalt/teal preset extraction preserves every value exactly, and the theme prop threading across all the QA/Showcase files is consistent throughout.
Requesting changes on process, not code:
This PR's description says "closes #64", but it's explicitly framed as a "first increment" and its own test plan lists remaining DoD items as follow-up work. Checking issue #64 directly, its Definition of Done has 3 items, and this PR only partially covers them:
- "All stories are accompanied with controls": this PR covers 4 widgets (StreamingWidget, CitizenClaimWidget, StakingMigrationWidget, ClaimWidget Theme Demo), but the repo has 6 widget areas with story files. ai-credits-widget and governance-widget both have QA + Showcase stories that aren't touched here.
- "All stories reviewed for clear guiding text": explicitly deferred per the PR's own test plan.
- "Component-level issues resolved or documented": partially done (the Stepper fix), no broader sweep.
Since this is tagged as a bounty issue, merging with "closes #64" as-is will auto-close the tracking issue before its DoD is actually met. Can we change this to "part of #64" and open a follow-up issue for the remaining scope (ai-credits-widget, governance-widget, and the human-eye review pass), or confirm with whoever manages the bounty that partial closure here is intentional?
Small nit: the PR description says "no QA fixture stories were touched", the three *QA.stories.tsx files were edited (just to add the new controls, no data-testid changes), so the underlying claim about stable test IDs holds, but the wording overstates it slightly.
| dark_SetStreamFormCard: { borderColor: '#00A884', shadowColor: 'rgba(0,168,132,0.65)' }, | ||
| }, | ||
| } | ||
|
|
There was a problem hiding this comment.
Every entry here is a dark_* key, there's no light_* equivalent for any of these (ClaimCard, StreamRow, PoolRow, BalanceCard, etc).
Since the QA/Showcase stories now expose defaultTheme and brandPreset as independent controls, selecting defaultTheme: light + a brand preset will apply the top-level tokens.color.primary globally, but none of the component-specific border/shadow/glow overrides will render, since those only exist for dark. That's the part that actually makes the preset visually "brand," so in light mode it'll look like the preset silently isn't working.
Is that intentional (presets are meant as dark-mode demos only), or should light variants be added here too?
|
@0xOlivanode the note on 'auto-closing', this is turned off in the repo so does not affect anything. |
3 participants
Summary
First increment of #64 (Polish storybook interaction and presentation). Focuses on the "controls" and "documentation" parts of the definition of done:
closes #64
argTypescontrols fordefaultThemeandthemeOverrides(via a shared "brand preset" select: None/Cobalt/Teal) across all four widget showcase stories — StreamingWidget, CitizenClaimWidget, StakingMigrationWidget, and ClaimWidget Theme Demo — so integrators can flip theme/branding live in the Controls panel instead of editing story code.claimWidgetStories.tsx) into a sharedthemeOverridePresets.tshelper reused by all widgets.argTypes+ aControllablestory to the Card, GlowCard, Stepper, and Drawer design-system primitives, following the existingTokenAmountpattern.CitizenClaimWidget.mdxandStakingMigrationWidget.mdxto matchClaimWidget.mdx's existing depth, plus a one-line pointer to the new controls on all three widget docs pages.Controllablestory'sactiveStepIdcontrol only auto-scrolled the list without changing which step rendered as "active" (status was hardcoded in a static array). Now recomputes step status relative to the selected step.All existing story ids /
data-testids used by the Playwright QA suite are preserved — no QA fixture stories were touched.Test plan
pnpm build(workspace packages) +pnpm build-storybooksucceeddefaultThemeandbrandPresetcontrols visibly change each widget's rendering (screenshots taken via a scripted headless pass), and that Card/Stepper/Drawer controls visibly change their primitivespnpm test:demo(Playwright) — 32 passed; 2 pre-existing failures instaking-migration-widget/states.spec.ts(ReadyStory/MigratingStory, untouched by this PR) appear to depend on live RPC access torpc.fuse.iounavailable in this sandbox — not caused by this change