Skip to content

design(WS7): Analyze Storybook pass - #277

Open
jdpigeon wants to merge 3 commits into
mainfrom
design/ws7-analyze
Open

jdpigeon wants to merge 3 commits into
mainfrom
design/ws7-analyze

Conversation

@jdpigeon

@jdpigeon jdpigeon commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Design pass for Workstream 7 (Analyze), following docs/uxr/2026-09-24-ws7-design-brief.md (copied into this branch) and plan §8.4, §9 and §10.2 "Analyze". This is Storybook only; runtime behavior does not change. Engineering integrates after approval. AnalyzeComponent.tsx, PyodidePlotWidget.tsx, AnalyzeContainer, the epics, the worker and compute.js are unchanged. One small shared edit is included: ClickableHeadDiagramSVG now destructures its props (details below).

Round 2 applies the product feedback. The layout is rebuilt around a controls rail. The topography fixture now matches what plot_topo really draws. The ERP tab puts the graph first and adds a four-step walkthrough built from epoch arrays. Behavior is plotted by the real aggregateDataForPlot.

Layout decision

Every tab uses one pattern: a 300px controls rail on the left and the results on the right. The page does not scroll. Plots shrink to fit their card instead of making the page taller. The shared pieces live in Analyze/AnalyzeParts.tsx: AnalyzeLayout, RailSection, DatasetChecklist, ResultStatus, CleanRequired, Segmented and PlotFigure.

  • PlotFigure wraps the unchanged PyodidePlotWidget in a scoped .bw-analyze-plot class in app.global.css. The class makes the image flex: 1; min-height: 0; object-fit: contain, so the image keeps its aspect ratio, fills the card, and Save as SVG/PNG stays underneath. No shared global class was changed.
  • Datasets are chosen from a checkbox list, one row per recording and led by the participant ID. It replaces <select multiple>, so choosing several recordings needs no Cmd/Ctrl.
  • Comparison, for product: CompareOverviewPlotsFirst puts the plots on top and a compact recordings strip below. One of it and OverviewResults gets deleted after review.

Measurements

Every story was loaded in Chromium at both sizes. The results:

  • Document scroll: 0 on every story.
  • Scrolling elements inside the story root: none. The rail never scrolls.
  • Horizontal overflow: none.
  • Console errors: 0.

"Plot bottom" is the lowest <figure> edge. "Rail content" is the lowest element inside the rail.

story 1366×768 plot bottom / rail content 1280×720 plot bottom / rail content
OverviewResults 748 / 437 (plot img 462px tall) 700 / 437 (414px)
CompareOverviewPlotsFirst 654, strip to 735 (388px) 606, strip to 687 (340px)
ErpResults (MNE plot) 586, walkthrough to 748 / 614 (320px) 538, to 700 / 614 (272px)
ErpWalkthroughStep1–4 586, walkthrough to 748 / 614 (chart 393px) 538, to 700 / 614 (chart 345px)
BehaviorResults, Export, ExportFailed, OnlyWorkspace, BeforeCleaningBehaviorTab 748 / 731 (Plotly 533px) 700 / 683 (485px)
Loading, Error and NoResult states rail 287 (Overview) / 614 (ERP) same
NoData, BehaviorBeforeCleaning, ErpCleanRequired no rail (blocked state), doc 0 same

Overview

  • The rail holds the cleaned recordings and an Included summary: participants, trials per condition (a palette dot plus the label), and "% of trials left out in cleaning", all taken from epochsInfo.
  • The results show "Power at each frequency" (PSD, fixture unchanged) and "ERPs across the scalp" side by side. Each has a one-line caption saying what it shows.
  • TOPO_PLOT_MIME now mirrors plot_topo (utils.py:163, MNE plot_evoked_topo). It draws a small ERP box at each Muse sensor's position on a head outline: AF7 and AF8 at the front, TP9 and TP10 at the ears. Each box has one line per condition in conditionPalette order, and the legend text is in the condition colors. The curves are computed from the same example epochs the ERP tab uses, so the two tabs agree.

ERP: graph first, lessons as a walkthrough

  • The rail holds the sensor picker. The head diagram (ClickableHeadDiagramSVG) and a 2×2 list of sensor buttons (aria-pressed) both select. An Image types legend shows a solid or dashed swatch, the label and the trial count.
  • The results area shows the graph, then a fixed 148px strip underneath.
  • Step 0 is the default results view. It is the static MNE erpPlot (ERP at TP9, bootstrapped bands, Save as SVG/PNG). The strip below invites the student in: "What am I looking at?" plus Walk me through it. I made MNE step 0 because it is the scientific result the app already computes and saves. The walkthrough is optional teaching layered on the same data. Exit and Finish both return to it.
  • Steps 1–4 replace the plot with ErpTraceChart, one step at a time in the style of ExploreLessonFlow: a "READING AN ERP · STEP n OF 4" label, gold progress pips, Exit walkthrough ✕, Back (outline) and Next / Finish (filled). The copy fades per step and the heading takes focus.
    1. Every line is one trial. All 80 trials are drawn faint and one is highlighted, with a pointer: "This is one trial".
    2. Average the trials together. Trials go fainter, the grand mean is drawn in ink, and a pointer says "The average of all 80 trials".
    3. Compare the two image types. The chart shows the Face and House means, solid versus dashed, with end labels spread so they never overlap. The copy says the scale zoomed in.
    4. Look around 170 ms. A shaded 130–210 ms window and a pointer read "Face may dip lower here, around 170 ms". The copy says "may… clearly, weakly or not at all" and never promises the effect.
  • Conditions are never told apart by color alone. They also differ by line style, direct labels and legend text.

Walkthrough data notes

  • EXAMPLE_EPOCH_ARRAYS in fixtures.ts has exactly the shape of pyodide.epochArrays: { buffer: ArrayBuffer; meta: EpochArraysMeta }, with a float32 C-order buffer laid out [epoch][channel][time].
    • 80 epochs (40 Face with code 1, 40 House with code 2, shuffled), 4 channels (TP9, AF7, AF8, TP10).
    • 231 samples from −0.1 to 0.8 s at 256 Hz, matching epochEvents(worker, eventId, -0.1, 0.8). Values are in µV and baseline-corrected.
  • The data is synthetic, and each trial has three parts:
    • background rhythms: 10 Hz and 5 Hz with random phase and amplitude;
    • sensor noise;
    • a small evoked response: P1 at 100 ms, then a dip at 170 ms that is deeper for faces on the ear-side sensors (weaker frontally), then a slow positivity.
    • Latency and gain jitter from trial to trial, and a seeded PRNG keeps the screenshots identical.
  • The chart computes every line from the buffer. It uses Clean's existing meanTrace and epochChannelSeries (CleanComponent/epochArrays.ts), so no mean is drawn by hand. The MNE-style erpPlotMime(channel) fixture and the topography fixture are built from the same arrays, so the plot title always matches the selected sensor.
  • EPOCHS_INFO agrees with the arrays: Face 40, House 40, 84 total, 4.76% dropped. The fixture therefore represents P01, and Overview opens with P01 ticked.
  • The fixture is labelled "example epochs" in the story descriptions only. The UI says "your trials".
  • Integration note: today epochArrays is only requested for raw_epochs (Clean's loadEpochsEpic). Analyze needs requestEpochArrays(worker, CLEAN_EPOCHS) after LoadCleanedEpochs. It also needs either its own state slot or a guarantee that Clean's reviewer is not showing at the same time, because they share pyodide.epochArrays.

Behavior

  • The rail holds four groups:
    • Recordings;
    • Plot: Response time | Accuracy and Bars | Dots | Box, as segmented controls where the choice is outlined in teal, not filled;
    • Remove outliers (with a one-line meaning) and Show data points;
    • Export.
  • Export summary CSV is the single filled action on the tab. Success and failure are both stated in words next to it (role="status").
  • The plot sits beside the rail. Its caption says what that plot type shows for the chosen measure.
  • The stories plot through the real aggregateDataForPlot (compute.js, imported read-only). Its input is example CSV rows shaped the way readBehaviorData returns them: Papa.parse output plus meta.datafile, with every value a string. So every control changes the plot exactly as it will in the app. compute.js is not modified.

Review agenda

Domain/Analyze, all rendered in the real AppShell with story-accurate badges and Next:

  • Workspace states: NoData (WorkspaceAreaGate's blocked state, Go to Collect) · BehaviorBeforeCleaning (Overview: clean required) · ErpCleanRequired · BehaviorBeforeCleaningBehaviorTab (Clean is Next, Behavior is fully usable) · BehaviorOnlyWorkspace (Behavior tab only, no Clean area)
  • Overview: OverviewResults · CompareOverviewPlotsFirst · OverviewLoading · OverviewError
  • ERP: ErpResults · ErpWalkthroughStep1 · Step2 · Step3 · Step4 · ErpNoResult · ErpLoading · ErpError
  • Behavior: BehaviorResults · BehaviorExport · BehaviorExportFailed

ErpExplainer has been removed; the walkthrough replaces it.

Open questions

  • Should the recordings panel sit in the rail (OverviewResults) or in a strip at the bottom (CompareOverviewPlotsFirst)?
  • ClickableHeadDiagramSVG has no selected state (only :hover fill) and its <title> reads "Signal Quality Indicator". The sensor buttons carry the selection today. Should integration give the diagram a selected prop and a correct accessible name?
  • Show data points does nothing. compute.js accepts showDataPoints but never reads it; Dots is the only way to see points. Should the checkbox be dropped, or should compute be fixed in a separate change?
  • Behavior plots use compute.js's hard-coded colors (#28619E / #3DBBDB), not conditionPalette, so Face is a different blue on Behavior than on ERP. Should this be aligned in a separate change?
  • Should the walkthrough also offer a fifth step that returns to the MNE plot, pointing out the confidence bands?
  • The walkthrough's 170 ms window is Faces/Houses-specific. For other experiments, should step 4 be hidden, or should it read the window from the experiment's params?

Stray-elements diagnosis (Playtest 1 P0: "Analyze layout broken; stray elements popping up")

These causes are in the current src/renderer/components/AnalyzeComponent.tsx, for engineering to fix during integration:

  1. :28 imports HelpButton from CollectComponent/LessonSidebar and never uses it. It is a leftover of the old sidebar.
  2. :79, :81, :285–327: the isSidebarVisible and helpMode help panel.
    • It renders in document flow at :595, between the tab bar and the content, not as a sidebar. Toggling it pushes the whole tab down; this is the element that "pops up".
    • It uses a bare ✕ button and an <h1> (:321) that the global h1 { 36px !important } inflates.
    • handleRemoveOutliers and handleDisplayModeChange change helpMode behind the scenes.
  3. :585–592: the "Show help / Hide help" ghost button goes into SecondaryNavComponent's saveButton slot, as size="icon" (36px square) holding text, so the label overflows its button. isEEGEnabled is not passed either, so the nav renders its legacy layout with a duplicate "Analyze" title.
  4. :409: the ERP channel column is w-32 (144px at the 18px root). ClickableHeadDiagramSVG has an inline minWidth: 250, so the head diagram overflows the column and overlaps the plot.
  5. :439–484: the Behavior tab repeats the "EEG Datasets" multi-select and the "No cleaned data yet… Go to Clean" block (:458, :476), so a cleaned-EEG picker and a Clean CTA appear inside Behavior. In a behavior-only workspace this is a Clean reference that should not exist.
  6. :579: the root is relative h-screen bg-app inside AppShell's flex-1 overflow-y-auto. It is a full window tall under a 64px bar, so it adds a second scroll region and clips the bottom.
  7. Global CSS. app.global.css forces h1 { 36px !important } and p { 18px !important }, and sets a large h2. The per-section <h1>s (:332, :398, :442) and <h2>PSD Plot</h2> / <h2>Topography</h2> (:365, :377) render oversized. The new components avoid bare <p>/<h1> or use scoped sizes. Separately, lab.css (layered) styles bare header, footer, main with 24px padding and centered text, so the integration should not wrap Analyze in semantic <main>/<header> without a scoped reset.

Constraints not fully met, and notes for integration

  • AnalyzeComponent.tsx is not replaced. Integration swaps its render branches for these components and removes items 1–6 above.
  • ClickableHeadDiagramSVG.tsx destructures its props instead of spreading them onto <svg>, which stops React's onChannelClick unknown-prop warning. This was accepted in round 1 and is otherwise unchanged.
  • PyodidePlotWidget's Save buttons still call preload storage, so they fail in Storybook if clicked. They are not clicked during verification.
  • The PSD fixture is still the original simple placeholder SVG. It is not a replica of MNE's compute_psd().plot().
  • The walkthrough needs epochArrays for cleaned epochs, which Analyze does not request today (see the integration note above).

Verification

  • npx tsc --noEmit: 0 errors.
  • All 20 stories were screenshotted at 1366×768 and 1280×720 (40 PNGs) in /tmp/ws7-design/{1366x768,1280x720}/. They were measured as in the table above: no document scroll, no inner scrollers, no horizontal overflow and 0 console or page errors on any story. The raw numbers are in /tmp/ws7-design/measurements.json.
  • Storybook: http://127.0.0.1:6009/?path=/story/domain-analyze--overview-results

- Add Analyze/fixtures.ts with dataset, epoch, channel and plot fixtures.
- Add AnalyzeOverview, AnalyzeErp, AnalyzeBehavior pure-props components.
- Add Analyze.stories.tsx with all required stories rendered inside the
  real AppShell chrome at location='analyze'.
- Fix ClickableHeadDiagramSVG prop spread so onChannelClick is not passed
  to the underlying <svg> element.
- Copy brief into docs/uxr.

Refs: docs/uxr/2026-09-24-ws7-design-brief.md, playtest plan §9/§10.2.
…ture

- Make AppShell badges/nextArea configurable per story via parameters.
- NoData: nextArea=collect, no badges.
- BehaviorBeforeCleaning: only Collect badge (4 recordings), nextArea=clean.
- EEG results stories: Collect + Clean badges, nextArea=analyze.
- Behavior-only workspace: Collect badge only, nextArea=analyze, Clean tab hidden.
- ErpResults: selectedChannel TP9 to match fixture SVG title.
- Re-screenshot all stories at 1366x768 and 1280x720; no overflow or console errors.
@jdpigeon

Copy link
Copy Markdown
Contributor Author

Review fixes (added after Main review)

  • BehaviorBeforeCleaning shell props: AppShell now shows only the collect: ['4 recordings'] badge and nextArea: 'clean', matching the state with complete behavior but no cleaned EEG. All stories now pass per-state badges and nextArea parameters:
    • NoData: no badges, nextArea: 'collect'.
    • BehaviorBeforeCleaning: { collect: ['4 recordings'] }, nextArea: 'clean'.
    • All EEG-results stories (Overview/ERP/Behavior tabs): { collect: ['4 recordings'], clean: ['3 cleaned'] }, nextArea: 'analyze'.
    • BehaviorOnlyWorkspace: { collect: ['4 recordings'] }, nextArea: 'analyze', Clean tab hidden.
  • ErpResults channel mismatch: selectedChannel is now "TP9" to match the fixture SVG title. The same applies to ErpExplainer.
  • Re-screenshot all 13 stories at both sizes; overflow and console-error checks remain clean.

…topo shape

- Controls rail + results on every tab; plots fit 1366x768 and 1280x720 without scroll.
- CompareOverviewPlotsFirst: plots on top, recordings strip below (review comparison).
- Topography fixture now mirrors plot_topo (per-sensor ERPs on a head), built from example epochs.
- ERP: graph and sensor picker first; 4-step walkthrough drawn as React SVG from
  EpochArraysMeta-shaped example epochs, averages computed with meanTrace.
- Behavior: rail holds recordings, plot options and export; plot computed by the
  real aggregateDataForPlot from example CSV rows; export success and failure stories.
- Scoped .bw-analyze-plot CSS lets PyodidePlotWidget images shrink to their card.
@jdpigeon

Copy link
Copy Markdown
Contributor Author

Round-2 review (Main): scope, measurements and screenshots checked. The ClickableHeadDiagramSVG change is still only the destructure fix. The new CSS is one scoped .bw-analyze-plot class. Averages reuse Clean's meanTrace.

Integration note, not a design change: the Behavior chart's two blues come from existing code. utils/behavior/compute.js:82 hard-codes colors = ['#28619E', '#3DBBDB'] instead of using conditionPalette, so Behavior disagrees with the ERP and Overview colors, and a Custom study with 3–4 conditions gets undefined colors for the extra conditions. Fix at integration: map conditions through cssColorForIndex.

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