feat: add Recharts adapter for accessible chart integration

[jooyoungseo]

Add a Recharts adapter that converts Recharts chart data and SVG
structure into MAIDR's accessible format. This enables audio
sonification, text descriptions, braille output, and keyboard
navigation for Recharts charts.

The adapter provides:

• MaidrRecharts wrapper component for convenience
• useRechartsAdapter hook for flexible integration with
• convertRechartsToMaidr utility for non-React usage
• CSS selectors targeting Recharts SVG elements for highlighting
• Support for bar, line, area, scatter, and pie chart types
• Simple mode (single chart type) and composed mode (mixed types)

Exported as maidr/recharts with full TypeScript type definitions.

Closes #542

https://claude.ai/code/session_01KDu3WPhNGkhRFYhY57xEKQ

[claude]

PR Review: Recharts Adapter

Good direction overall — this fills a real gap for Recharts users who want MAIDR accessibility. The API surface (simple mode vs composed mode, MaidrRecharts wrapper + useRechartsAdapter hook + convertRechartsToMaidr utility) is well-thought-out and the documentation/JSDoc is thorough. A few issues need to be addressed before merging.

---

Critical

1. useRechartsAdapter memoization is effectively broken inside MaidrRecharts

useRechartsAdapter uses useMemo(() => ..., [config]), but MaidrRecharts always passes a freshly constructed object literal:

// MaidrRecharts.tsx — new object created on every render
const maidrData = useRechartsAdapter({
  id, title, subtitle, caption, data, chartType,
  xKey, yKeys, layers, xLabel, yLabel, orientation,
});

Because config is a new reference each render, the memo never hits its cache. The conversion runs on every render, and the <Maidr> component receives a new data object every time — potentially triggering unnecessary re-initialization.

Fix: Memoize with the primitive props as dependencies rather than wrapping them in an object first:

// In MaidrRecharts.tsx
const maidrData = useMemo(
  () => convertRechartsToMaidr({ id, title, subtitle, caption, data, chartType, xKey, yKeys, layers, xLabel, yLabel, orientation }),
  [id, title, subtitle, caption, data, chartType, xKey, yKeys, layers, xLabel, yLabel, orientation],
);

The companion warning in the hook's JSDoc ("keep the config object reference stable") is good but insufficient when the hook is hidden behind MaidrRecharts.

---

2. :nth-of-type selector bug for multi-series charts

selectors.ts uses :nth-of-type to scope a selector to a specific series:

// getNthSeriesPrefix
// produces e.g.: ".recharts-bar:nth-of-type(2) .recharts-bar-rectangle"

CSS :nth-of-type filters by element tag name, not by class. Since Recharts series render as <g> elements, g.recharts-bar:nth-of-type(2) selects the 2nd <g> element inside the parent regardless of its class — meaning axes, grids, and other <g> siblings shift the count and the wrong series gets highlighted.

Recommended fix: Use :nth-child with a class-specific filter, or — more robustly — iterate querySelectorAll('.recharts-bar') results in JavaScript and index into them directly, rather than relying on CSS positional selectors. This is the most likely cause of highlighting failures in real multi-series charts and should be verified against actual Recharts-rendered SVG before merging.

---

Major

3. Pie chart mapped to TraceType.BAR

// converters.ts — toTraceType()
case 'pie':
  return TraceType.BAR;

There is no PIE value in the TraceType enum, so this fallback is understandable, but it produces bar-chart navigation semantics (arrow keys move through categories linearly) and bar-chart text descriptions ("Bar 1 of 5") for pie sectors, which will likely confuse screen reader users.

Options:

• Document explicitly that pie renders as categorical bars and surface this in the type definitions/JSDoc
• Defer pie support until a proper trace type exists
• Open a follow-up issue to add TraceType.PIE

---

4. No tests

817 lines of new public API with zero test coverage. The converters in particular are pure functions that are straightforward to unit-test:

describe('convertRechartsToMaidr', () => {
  it('converts simple bar config', () => {
    const result = convertRechartsToMaidr({
      id: 'c',
      data: [{ x: 'A', y: 1 }],
      chartType: 'bar',
      xKey: 'x',
      yKeys: ['y'],
    });
    expect(result.subplots[0][0].layers[0].type).toBe(TraceType.BAR);
  });
});

The selector logic (getRechartsSelector) and composed-mode layer building should be covered as well, consistent with the project's existing test patterns.

---

5. Missing recharts peer dependency declaration

The adapter is positioned as a Recharts integration (maidr/recharts), but recharts is absent from peerDependencies. Even though the adapter itself does not import from 'recharts', consumers must have Recharts installed for the CSS selectors and data structure assumptions to be valid. Package managers won't warn users if they omit it.

Suggestion:

"peerDependencies": {
  "recharts": "^2.0.0"
},
"peerDependenciesMeta": {
  "recharts": { "optional": true }
}

---

Minor

6. Unnecessary non-null assertion

// converters.ts — buildComposedLayers
const layerConfigs = config.layers\!;

buildComposedLayers is only called from buildLayers when config.layers is truthy, so TypeScript will narrow correctly. The \! is redundant and can be removed.

---

7. Silent toNumber returning 0 for unrecognized values

function toNumber(value: unknown): number {
  if (typeof value === 'string') {
    const num = Number(value);
    return Number.isNaN(num) ? 0 : num;
  }
  return 0;
}

Silently returning 0 when a value is non-numeric means a mistyped key name produces a chart full of zeros with no feedback. At minimum, a console.warn when the value is neither a number nor a parseable string would make this much easier to debug.

---

8. area mapping is undocumented in user-facing types

Mapping 'area' to TraceType.LINE is reasonable (area is a filled line trace), but users will hear "line chart" descriptions when navigating an area chart. This should be noted explicitly in the RechartsChartType JSDoc so users know what accessibility output to expect.

---

Summary

Severity	Issue
Critical	MaidrRecharts constructs a new config object every render, breaking useMemo
Critical	:nth-of-type targets element tag not class — multi-series highlighting will be incorrect
Major	Pie chart uses BAR semantics without documentation or deferral
Major	No tests for 817 lines of new code
Major	recharts missing from peerDependencies
Minor	Unnecessary \! non-null assertion in buildComposedLayers
Minor	toNumber silently zeroes bad data without any warning
Minor	area to LINE mapping not surfaced in user-facing documentation

[claude]

PR Review: Recharts Adapter (updated review based on current code)

The adapter is well-structured and addresses a real need. Many issues flagged in the earlier review have already been addressed (memoization in MaidrRecharts, test coverage, and the recharts peer dependency). Here is a review of the current code.

---

Critical

1. :nth-child selector is broken for real-world Recharts SVG

selectors.ts generates selectors like .recharts-bar:nth-child(1) .recharts-bar-rectangle to scope to a specific series. However, CSS :nth-child(n) counts all siblings of the parent, regardless of class. A typical Recharts SVG has many <g> elements before the first series — CartesianGrid, XAxis, YAxis, reference lines, etc.:

<g class="recharts-surface">
  <g class="recharts-cartesian-grid">   <!-- child 1 -->
  <g class="recharts-xAxis">            <!-- child 2 -->
  <g class="recharts-yAxis">            <!-- child 3 -->
  <g class="recharts-layer recharts-bar"> <!-- child 4 — series 0 -->
  <g class="recharts-layer recharts-bar"> <!-- child 5 — series 1 -->

With this structure, .recharts-bar:nth-child(1) matches nothing (the first child is a grid, not a bar). The tests pass because they only verify the generated string, not that it actually selects elements in a browser.

Note: The CSS Selectors 4 :nth-child(n of .selector) syntax (which counts only matching elements) would fix this, but its browser support is not yet universal.

Recommended fix: Abandon CSS positional selectors for multi-series selection. Instead, select all containers of a type and index in JavaScript:

// Return a selector + series index to the highlight service
// and let it do: querySelectorAll('.recharts-bar')[seriesIndex].querySelectorAll(...)

Or document clearly that highlighting is best-effort for multi-series charts and may degrade in composed layouts.

---

Major

2. useRechartsAdapter hook has a memoization footgun

The hook memos on the config object reference:

export function useRechartsAdapter(config: RechartsAdapterConfig): Maidr {
  return useMemo(() => convertRechartsToMaidr(config), [config]);
}

The documentation says to keep the config reference stable, but the idiomatic usage pattern in the JSDoc shows an inline object:

const maidrData = useRechartsAdapter({
  id: 'sales-chart',
  // ...  ← new object every render, memo never hits cache
});

MaidrRecharts correctly uses useMemo with individual props, but the standalone hook will silently skip memoization for most users. Consider accepting a dependency array as a second argument, or spreading individual props internally like MaidrRecharts does, or removing the useMemo from the hook entirely and documenting that the caller is responsible for memoization.

---

3. Line chart x-values are coerced to numbers; bar chart x-values are not

convertToLinePoints uses toNumber(item[xKey]), while convertToBarPoints uses item[xKey] as string | number:

// Bar: preserves strings
{ x: item[xKey] as string | number, y: toNumber(item[yKey]) }

// Line: coerces to number
{ x: toNumber(item[xKey]), y: toNumber(item[yKey]) }

A line chart with a date/category x-axis (e.g., 'Jan', 'Feb', 'Mar') will coerce those labels to 0, producing a flat x-axis with no useful navigation descriptions. This should be kept consistent — preserve the original value for x, or at least document that line x-axis must be numeric.

---

4. stacked_bar / dodged_bar / normalized_bar with a single yKey produces an invalid layer

In buildSimpleLayers, segmented bar handling only triggers when hasMultipleSeries is true:

if (isSegmentedBarType(chartType) && hasMultipleSeries) {
  return [buildSegmentedBarLayer(config, maidrType)];
}

With a single yKey, execution falls through to the single-series path, which calls convertData('stacked_bar', ...). That returns BarPoint[] data, but the layer gets TraceType.STACKED, creating a type/data mismatch. The correct behavior is probably to treat it as a plain TraceType.BAR, or throw an error, since a stacked bar with one series is meaningless.

---

Minor

5. pie and funnel mapped to TraceType.BAR without documentation

Users will hear "Bar 1 of 5" style descriptions when navigating a pie or funnel chart. This is understandable as a workaround, but the RechartsChartType JSDoc is the right place to note it explicitly:

/**
 * - 'pie' → TraceType.BAR — Pie sectors navigated as bar categories.
 *   Screen reader will announce "bar" terminology.
 */

The existing comment mentions it at a high level, but surfacing it on the type definition itself will prevent confusion.

---

6. toNumber silently returns 0 for invalid or missing values

function toNumber(value: unknown): number {
  if (typeof value === 'string') {
    const num = Number(value);
    return Number.isNaN(num) ? 0 : num;
  }
  return 0;
}

A mistyped key (e.g., yKeys: ['revnue'] instead of 'revenue') produces a silent chart of zeros. A console.warn when the value is neither a number nor a parseable string would make this much easier to debug.

---

7. Unnecessary non-null assertion in buildComposedLayers

return layers.map((layerConfig: RechartsLayerConfig, index: number) => {
  // ...
  const layerData = convertData(chartType, data, xKey, yKey);

Not an assertion issue here, but in buildSegmentedBarLayer:

return yKeys!.map((yKey, i) => ({  // ← non-null assertion

The caller (buildSimpleLayers) already validates yKeys is non-empty before reaching this path. The ! can be dropped by narrowing the function signature to accept yKeys: string[] instead of reading from config.

---

Summary

Severity	Issue
Critical	:nth-child multi-series selectors won't match in real Recharts SVG due to sibling element ordering
Major	useRechartsAdapter hook memos on object reference — memoization is effectively no-op for inline configs
Major	Line chart x-values coerced to numbers; string category labels become 0
Major	Stacked/dodged/normalized with single yKey creates TraceType.STACKED + BarPoint[] mismatch
Minor	Pie/funnel mapped to BAR without surfacing this in per-type JSDoc
Minor	toNumber returning 0 silently for invalid values
Minor	Unnecessary non-null assertion in buildSegmentedBarLayer

[claude]

PR Review: feat: add Recharts adapter for accessible chart integration

Overall this is a well-structured, thoroughly-documented addition that follows the project's patterns cleanly. The separation of the adapter outside the core MVVC layers is the right call, and the test coverage is excellent. Below are observations ranging from bugs to suggestions.

---

Bug: Composed Mode Always Disables CSS Highlighting

In buildComposedLayers, every layer gets seriesIndex = typeCounters.get(chartType) ?? 0 — which is 0 for the first occurrence. Then getRechartsSelector(chartType, seriesIndex) is called with seriesIndex = 0.

Inside getRechartsSelector:

if (seriesIndex \!= null) {  // 0 \!= null → true
  return undefined;
}

Because 0 \!= null evaluates to true in JavaScript, every layer in composed mode gets undefined selectors, including charts with only a single layer of a given type. The test even documents this outcome but frames it as expected behavior. In practice this means composed mode silently loses all visual highlighting.

A minimal fix: only disable highlighting when seriesIndex > 0 (i.e. there are multiple series of the same type):

if (seriesIndex \!= null && seriesIndex > 0) {
  return undefined;
}

Or, alternatively, don't pass seriesIndex for the first occurrence of each type.

---

Concern: Unreachable Cases in convertData Are Misleading

convertData has a return type of BarPoint[] | LinePoint[][] | ScatterPoint[] but includes cases for 'stacked_bar', 'dodged_bar', 'normalized_bar', and 'histogram' that should never be reached under normal flow (they're handled by dedicated builders in buildSimpleLayers). The current fallback silently returns BarPoint[] for segmented bar types, which would be incorrect data if those code paths were ever hit.

Consider removing the dead cases and adding a default exhaustiveness check, or at minimum adding a comment and/or throw for those paths.

---

Concern: Silent Data Coercion May Hide Data Issues

toNumber() silently returns 0 for null, undefined, non-numeric strings, and booleans. This means bad data looks valid to the MAIDR layer. For example, { x: 'A', y: null } in a scatter chart becomes { x: 0, y: 0 } with no warning.

A console.warn in development, or explicit null-handling at the call site (e.g. filtering null points), would make data problems easier to diagnose — especially important for an accessibility tool where incorrect data can lead to misleading audio/text output.

---

Minor: Relative Import in MaidrRecharts.tsx Is Fragile

import { Maidr } from '../../maidr-component';

All other files use path aliases (@type, @state, etc.). This relative path will break silently if the adapter directory depth changes. Consider an alias like @root/maidr-component or exporting through an existing alias.

---

Minor: Missing Tests for React Layer

The test suite covers converters.ts and selectors.ts very well. However, MaidrRecharts.tsx and useRechartsAdapter.ts have no tests. At minimum a smoke test verifying the memoized hook returns the expected Maidr shape and that MaidrRecharts renders without throwing would strengthen confidence.

---

Minor: vite.recharts.config.ts — Overly Broad process.env Override

define: {
  'process.env': {},
},

This replaces the entire process.env object with an empty one. If any transitive dependency (now or in the future) reads a specific env key it will get undefined rather than failing visibly. A more targeted define (e.g. 'process.env.NODE_ENV': JSON.stringify('production')) would be safer.

---

Nit: pie and funnel Map to TraceType.BAR Without Issue Link

The comment in types.ts mentions that pie/funnel use bar semantics because MAIDR doesn't have dedicated types yet. It would be helpful to reference the tracking issue (e.g. a GitHub issue number) so this limitation is visible in the code and easier to follow up on.

---

Nit: id Required But Other Metadata Optional

RechartsAdapterConfig.id is required, but Maidr.id usage in the core library should be verified to accept it as-is. Worth a quick check that the generated MAIDR data's id field aligns with any DOM ID constraints (e.g. uniqueness across multiple charts on the same page — is the consumer expected to ensure this?). If so, document it explicitly.

---

Strengths Worth Calling Out

• The simple/composed mode split is clean and the mutual-exclusivity is validated with helpful error messages.
• Documenting the multi-series CSS limitation clearly in selectors.ts and providing selectorOverride as an escape hatch is the right design.
• useMemo is correctly applied, with each config field individually listed in the dependency array rather than using the config object reference.
• Test coverage for error paths (empty layers, missing chartType/yKeys) is thorough.
• Version bump (3.50.0 → 3.51.0) follows semver correctly for a new feature.

---

Summary: The composed-mode CSS selector bug (#1) is the most impactful issue and should be fixed before merge. The data coercion concern (#3) is worth addressing given this is an accessibility tool. The rest are minor.