Skip to content

Add colourblind-safe palette toggle with marker shapes - #131

Open
eurunuela wants to merge 2 commits into
ME-ICA:masterfrom
eurunuela:feature/colourblind-palette
Open

Add colourblind-safe palette toggle with marker shapes#131
eurunuela wants to merge 2 commits into
ME-ICA:masterfrom
eurunuela:feature/colourblind-palette

Conversation

@eurunuela

Copy link
Copy Markdown
Collaborator

Ports the colourblind report encoding from ME-ICA/tedana#1475 to rica.

What

  • Central palette module src/constants/palette.js — a single source of truth for classification styling, replacing the getColors(isDark) block that was copy-pasted across five files. Holds both the current pastel palette and the Okabe-Ito colourblind-safe palette (accepted #009E73, rejected #D55E00, ignored #0072B2, other #999999), plus a per-class marker shape. Unknown classifications fall back to other.
  • Colourblind toggle — a persisted flag on ThemeContext (localStorage rica-colorblind), surfaced as a header button next to the light/dark toggle. Opt-in; default look unchanged.
  • Marker shapes in the scatter plots (circle = accepted, square = rejected, triangle = ignored, diamond = other) as a redundant, non-colour cue. Shapes apply always, regardless of palette; only the colours respond to the toggle.
  • Refactored ScatterPlot, PieChart, ComponentTable, Plots, and DecisionTree to consume the shared module.

Notes

  • Consumers read the colorblind flag via useTheme() rather than prop-threading through the many render sites.
  • ComponentTable badge/row and DecisionTree nodes previously lumped ignored in with the rejected colour; routing through the module now gives ignored its own blue everywhere — a small intended consistency change.
  • Net −98 / +201 lines, but the production bundle shrinks ~8 kB from removing the duplicated palettes.

Verification

  • New unit test src/constants/palette.test.js (Okabe-Ito exactness, distinct colours/shapes, other fallback, shape independent of the flag). Full suite passes; react-scripts build is clean.
  • Verified live in the app with a synthetic report: both palettes render correctly across scatter, rank plots, pie, and time series, with shapes preserved when toggling.

🤖 Generated with Claude Code

Port tedana's colourblind report encoding (ME-ICA/tedana#1475) to rica.
Introduce a single source of truth for classification styling in
src/constants/palette.js holding both the default pastel palette and the
Okabe-Ito colourblind-safe palette, plus a per-class marker shape.

- Add a persisted "colourblind" toggle to ThemeContext (localStorage
  'rica-colorblind'), surfaced as a header button beside the theme toggle.
- Marker shapes (circle/square/triangle/diamond) always encode class in the
  scatter plots, independent of palette, as a redundant non-colour cue.
- Refactor ScatterPlot, PieChart, ComponentTable, Plots and DecisionTree to
  consume the shared module instead of duplicated inline getColors() blocks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 22, 2026 15:44
@netlify

netlify Bot commented Jul 22, 2026

Copy link
Copy Markdown

Deploy Preview for rica-fmri ready!

Name Link
🔨 Latest commit f7d6423
🔍 Latest deploy log https://app.netlify.com/projects/rica-fmri/deploys/6a60ea675f97350008f4eed7
😎 Deploy Preview https://deploy-preview-131--rica-fmri.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a centralized classification styling system (colors + redundant marker shapes) and a user-facing toggle to switch to an Okabe–Ito colourblind-safe palette, then refactors existing visual components to consume the shared palette API via useTheme().

Changes:

  • Added src/constants/palette.js (default + colourblind palettes, per-class marker shapes) plus unit tests.
  • Added a persisted colorblind flag to ThemeContext and a header toggle button to control it.
  • Refactored DecisionTree + plot components (scatter, pie, tables, time series colours) to use colorFor(...) / getClassStyle(...).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/constants/palette.js New single source of truth for class colors (default + Okabe–Ito) and marker shapes.
src/constants/palette.test.js Unit tests verifying palette exactness, fallback behavior, and selection variants.
src/index.js Adds persisted colorblind state + header toggle; injects new context values.
src/Tree/DecisionTree.js Switches accepted/rejected and time series line colors to colorFor(...); reads colorblind from theme.
src/Plots/ScatterPlot.js Replaces circle-only markers with per-class shapes; uses shared class styling.
src/Plots/PieChart.js Uses shared palette for slice colors (including selected/hover variants).
src/Plots/ComponentTable.js Uses shared palette for row/badge highlighting, now driven by classification.
src/Plots/Plots.js Uses shared palette for toggle/time series/spectrum line colours; reads colorblind from theme.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Tree/DecisionTree.js Outdated
Comment thread src/Tree/DecisionTree.js
Comment thread src/Plots/ScatterPlot.js Outdated
Comment thread src/Plots/Plots.js Outdated
Comment thread src/Plots/PieChart.js Outdated
Comment thread src/Plots/ComponentTable.js Outdated
Comment thread src/index.js
Comment thread src/Tree/DecisionTree.js
- Extract ThemeContext/useTheme into src/contexts/theme.js so components no
  longer import from the app entrypoint (index.js has module-level render side
  effects); fixes the circular-dependency review comments.
- Give the colourblind palette lightened dark-mode variants so classes keep
  their visual punch on the dark background (all >= 3:1 contrast).
- Add aria-label + type="button" to the colourblind toggle for screen readers.
- Document why DecisionTree uses the saturated shade (colours are rendered as
  text there, not fills).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@eurunuela

Copy link
Copy Markdown
Collaborator Author

Thanks for the review! Addressed in f7d6423:

  • Circular dependency (5 comments): extracted ThemeContext/useTheme into src/contexts/theme.js; index.js and all consumers now import from there instead of the app entrypoint.
  • Toggle button a11y: added aria-label + type="button".
  • DecisionTree selected: true (dark shade): kept intentionally and documented with a comment — the tree renders the classification as coloured text (not a fill like the scatter), so it needs the saturated shade to stay legible in both themes; the pale base is nearly unreadable on white.
  • ignored in the tree: correct as-is — the tree only has accept/reject decision outcomes, so there's no "ignored" node state. Corrected the PR description, which had overstated this.

Also added lightened dark-mode variants to the colourblind palette so the classes keep their punch on the dark background (all ≥ 3:1 contrast), with a test covering them.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/Tree/DecisionTree.js:299

  • selectedClassification currently collapses any non-"accepted" final classification into "rejected". This means components that end as "ignored", "unclassified", or other intermediate values (e.g. provisional states) will be styled with the rejected colour, which breaks the intended palette/shape consistency (and the colourblind-safe mapping). Consider normalizing the final classification string into one of {accepted,rejected,ignored,other} (e.g. by substring match on accept/reject/ignore) before passing it to colorFor.
    if (!selectedComponent || !componentPaths[selectedComponent]) return "accepted";
    const path = componentPaths[selectedComponent];
    const finalClass = path.nodes[path.nodes.length - 1]?.classification || path.initial;
    return finalClass === "accepted" ? "accepted" : "rejected";
  }, [selectedComponent, componentPaths]);

Comment thread src/Plots/ScatterPlot.js
@eurunuela
eurunuela requested a review from tsalo July 22, 2026 18:26
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.

2 participants