feat: add AnyChart charting library adapter (#545)

[jooyoungseo]

Add a new maidr/anychart entry point that provides an adapter for
integrating AnyChart charts with MAIDR's accessible visualization
features. The adapter extracts series data and metadata from drawn
AnyChart chart instances and generates the MAIDR JSON schema.

• Add bindAnyChart() and anyChartToMaidr() public API functions
• Add TypeScript type definitions for the AnyChart API subset used
• Support bar, column, line, area, scatter, box, heatmap, and
  candlestick series type mapping
• Add Vite build config for the new entry point
• Add ./anychart package export with types
• Add example HTML demonstrating AnyChart bar chart integration

Closes #545

https://claude.ai/code/session_018asBR8TGBxyy9oCrkQ7Yhk

[claude]

PR Review: feat: add AnyChart charting library adapter (#545)

Thank you for adding adapter support for AnyChart — this is a valuable addition for chart library integrations. I have reviewed the code carefully and found a few issues ranging from critical bugs to minor style concerns.

---

Critical Issues

1. Runtime never handles the maidr:bindchart event

The bindAnyChart() function sets the maidr-data attribute and dispatches a maidr:bindchart custom event. However, the MAIDR runtime (src/index.tsx) never listens for this event. The runtime main() function runs exactly once (at DOMContentLoaded or immediately if already complete), scans the DOM for [maidr-data] attributes, and then exits. Since chart.draw() and bindAnyChart() necessarily happen after DOMContentLoaded (the chart container must exist first), the attribute is set too late — main() has already run and found nothing.

The maidr:bindchart event is fired into the void because no listener is registered anywhere in the MAIDR runtime:

// src/index.tsx — no listener for maidr:bindchart exists anywhere
if (document.readyState === 'loading') {
  document.addEventListener(DomEventType.DOM_LOADED, main);
} else {
  main();
}

Impact: bindAnyChart() will appear to succeed (returns a Maidr object), but MAIDR will never initialise accessibility features for the chart.

Fix options:

• Have the MAIDR runtime listen for maidr:bindchart on document and call initMaidr()
• Or expose initMaidr publicly so bindAnyChart() can call it directly
• Or document that users must manually integrate the returned Maidr object with initMaidr()

The example HTML has the same problem: MAIDR script runs before bindAnyChart() is called, so the runtime has already scanned the DOM and found no charts.

---

2. BOX, HEATMAP, and CANDLESTICK series silently produce wrong data structures

mapSeriesType() correctly maps box -> TraceType.BOX, heatmap/heat -> TraceType.HEATMAP, and candlestick -> TraceType.CANDLESTICK. However, the switch in anyChartToMaidr() only handles LINE and SCATTER explicitly — all other types (including BOX, HEATMAP, CANDLESTICK) fall through to buildBarLayer():

switch (traceType) {
  case TraceType.LINE:
    layer = buildLineLayer(series, i, options?.selectors);
    break;
  case TraceType.SCATTER:
    layer = buildScatterLayer(series, i, options?.selectors);
    break;
  default:
    // BOX/HEATMAP/CANDLESTICK also land here incorrectly
    layer = buildBarLayer(series, i, options?.selectors);
    break;
}

The data structures MAIDR expects are completely different:

• BoxPoint requires { fill, lowerOutliers, min, q1, q2, q3, max, upperOutliers }
• HeatmapData requires { x: string[], y: string[], points: number[][] }
• CandlestickPoint requires { value, open, high, low, close, volume, trend, volatility }

These are silently produced as BarPoint[] instead, which will cause downstream failures (wrong audio, incorrect navigation, potential runtime errors in the trace models). Either add the missing layer builders (buildBoxLayer, buildHeatmapLayer, buildCandlestickLayer) or skip unsupported types with a console.warn rather than silently falling back to an incompatible structure.

---

Major Issues

3. CSS selectors almost certainly will not match AnyChart DOM

buildSeriesSelector(seriesIndex) generates a selector using .anychart-series-N class names. AnyChart SVG DOM does not use these class names — it uses internal class names with renderer-specific identifiers. This selector will match nothing, making SVG element highlighting non-functional for all users. The comment acknowledges this is a guess ('A reasonable default'), but it should be validated against AnyChart actual DOM or removed in favour of requiring users to supply options.selectors explicitly.

4. selectors option incorrectly shared across all series

When options.selectors is provided, the same value is applied to every layer in a multi-series chart:

layer = buildBarLayer(series, i, options?.selectors);  // same value for all i

This means every series targets the same SVG elements. Per-series selector overrides would need a different API shape such as selectors: string | string[] | Array<string | string[]> or a callback indexed by series.

---

Minor Issues

5. Unnecessary double call to resolveContainerElement

anyChartToMaidr() calls resolveContainerElement(chart) solely to extract the container id. Then bindAnyChart() calls it a second time. If the container changes between calls (unlikely but possible), the two calls could yield different elements. Consider passing the resolved element between functions or accepting it as a parameter.

6. Inconsistent key casing in mapSeriesType

The mapping mixes kebab-case and snake_case for multi-word keys:

'step-line': TraceType.LINE,   // kebab-case
'step-area': TraceType.LINE,   // kebab-case
'spline_area': TraceType.LINE, // snake_case — inconsistent

Verify what AnyChart seriesType() actually returns for spline area and normalise accordingly.

7. No tests

Per the project code review checklist in CLAUDE.md, tests must pass before merging. This PR adds ~640 lines of new logic with zero test coverage. Unit tests for mapSeriesType(), anyChartToMaidr(), and the data extraction functions would be a baseline expectation.

8. Debug console.log in example file

examples/anychart-bindable.html includes a console.log for generated MAIDR data. This is fine during development but should be removed or commented out in the final example.

---

What Works Well

• TypeScript types for the AnyChart API subset are intentionally narrow — good practice to avoid bundling AnyChart as a hard dependency.
• extractTitle and extractAxisTitle are wrapped in try/catch for resilience against AnyChart API variations.
• resolveContainerElement handles all three possible return types of chart.container() (string id, HTMLElement, Stage-like object).
• The two-function public API (anyChartToMaidr + bindAnyChart) gives users a clean escape hatch if they want the data object without the DOM mutation side-effect.
• Build config follows the same pattern as vite.react.config.ts.
• Version bump (3.50.0 to 3.51.0) is appropriate for new functionality.

---

Summary: The critical integration bug (issue 1) means this adapter will not work at all in its current form — bindAnyChart() fires an event no listener will ever handle. The silent data structure mismatch for BOX/HEATMAP/CANDLESTICK (issue 2) would cause subtle failures for those chart types. I recommend addressing those two before merging.

[claude]

PR Review: feat: add AnyChart charting library adapter

HIGH PRIORITY

1. Silent accuracy limitation: box plot outliers always empty
   buildBoxLayer hardcodes lowerOutliers/upperOutliers as empty arrays. AnyChart exposes outlier data through series statistics. Silently omitting outliers gives screen-reader users an inaccurate chart description. Either extract the outliers or emit a console.warn and document the limitation in JSDoc.

2. Double-initialization not guarded
   bindAnyChart unconditionally overwrites the maidr-data attribute and re-dispatches maidr:bindchart. Calling it twice on the same chart (easy in a re-render cycle) creates duplicate keyboard handlers and observers. Add a guard: check whether the element already has maidr-data set before proceeding.

3. Area series silently mapped to LINE
   area, step-area, and spline-area are converted to TraceType.LINE without any warning. A sighted user sees a filled area chart; a screen-reader user hears a line description. Document this in JSDoc and emit a console.warn at runtime so developers are aware of the semantic downgrade.

MEDIUM PRIORITY

4. bindAnyChart is untested
   anyChartToMaidr has broad test coverage, but bindAnyChart (DOM mutation + custom event dispatch) has zero. Add tests for: maidr-data attribute being set on the container, maidr:bindchart being dispatched, null return when the container cannot be resolved, and the double-init guard once added.

5. Stale event listener in src/index.tsx
   The maidr:bindchart listener is registered at module load and never removed. The rest of the codebase uses the Disposable pattern rigorously. Register this listener through the same lifecycle as other document-level listeners, or document why it is intentionally permanent.

6. bar and column both map to TraceType.BAR
   In AnyChart, bar = horizontal bars and column = vertical bars. If MAIDR ever needs to differentiate orientation for braille layout or text descriptions, this mapping will need to change. A comment noting the deliberate collapse is worthwhile.

7. vite.anychart.config.ts: define replaces all of process.env
   define: { process.env: {} } replaces the entire object. Any code (including dependencies) reading process.env.NODE_ENV gets undefined instead of production. Safer: 'process.env.NODE_ENV': JSON.stringify('production').

LOW PRIORITY

8. AnyChartBinderOptions.selectors type is redundant
   The union string | string[] | Array<string | string[] | undefined> is redundant because the third form subsumes the first two. Simplify to Array<string | string[] | undefined> and add a JSDoc example.

9. resolveContainerElement does not handle string container IDs
   AnyChart allows chart.container('my-div-id') with a string element ID. If chart.container() returns a string, fall back to document.getElementById(). Currently a string return silently produces null with no indication why.

10. Example file: outdated type=text/javascript attribute
    The script tag type=text/javascript is unnecessary in HTML5 and inconsistent with the adjacent ES module script.

11. console.warn vs. project logger
    The adapter uses raw console.warn. Using a project logger utility would make output controllable in tests/production.

WHAT IS DONE WELL

• MVVC boundary respected: no imports from /service/, /state/viewModel/, or /ui/.
• Minimal type file avoids pulling in the entire AnyChart SDK, keeping the bundle lean.
• mapSeriesType normalises casing and separators before lookup.
• The custom maidr:bindchart event is a clean extension point decoupled from MAIDR internals.
• anyChartToMaidr test coverage is broad: bar, line, scatter, box, heatmap, candlestick, histogram, multi-series, and options overrides.
• Candlestick trend detection (Bull/Bear/Neutral) is a nice accessibility-improving detail.
• extractRawRows resets the iterator before traversal to avoid re-use bugs.

Reviewed against CLAUDE.md: MVVC architecture, KISS principle, Disposable pattern, observer/emitter conventions.

[claude]

PR Review: AnyChart Adapter (#557)

This is a well-structured addition that follows a clean adapter pattern. The documentation is excellent, the type definitions are narrow and intentional, and the test coverage is solid. Below are issues ranging from bugs to nits.

---

Bugs / Correctness

1. Histogram builder is unreachable dead code

mapSeriesType has no entry for 'histogram', so buildHistogramLayer (and the TraceType.HISTOGRAM branch in buildLayer) can never be reached through the normal anyChartToMaidr flow. The test even acknowledges this with a comment:

// Since there's no 'histogram' seriesType in AnyChart, we test the builder
// through a direct call to anyChartToMaidr
// For now, test the BAR output from column.

Either add 'histogram' to the mapping (if AnyChart exposes it), or remove buildHistogramLayer and the dead case TraceType.HISTOGRAM branch. Dead code with a test that sidesteps the actual entry path is misleading.

2. buildLayer's default silently falls back to BAR

default:
  return buildBarLayer(series, seriesIndex, selectors); // silent fallback

This masks programming errors. Any unhandled TraceType variant — including future ones — will silently emit a bar layer with wrong data. Prefer an exhaustive check or throw:

default:
  throw new Error(`[maidr/anychart] No layer builder for TraceType: ...`);

3. JSON.parse without error handling on double-call path

if (boundElements.has(container)) {
  const existing = container.getAttribute('maidr-data');
  return existing ? JSON.parse(existing) as Maidr : null; // can throw
}

If the attribute is tampered with between calls (or malformed), this throws an uncaught exception. Wrap in try/catch and return null on failure.

4. Histogram bin widths are hardcoded to ±0.5

xMin: x - 0.5,
xMax: x + 0.5,

This is arbitrary and will be wrong for any histogram whose bins are not exactly 1 unit wide. Actual bin boundaries should be derived from the data (e.g., half-distance to adjacent bin centers). Even if this is a known limitation, it should emit a warning.

5. Smooth trace emits placeholder SVG coordinates

const points: SmoothPoint[] = rows.map(r => ({
  x: asNumber(r.x),
  y: asNumber(r.value ?? r.y),
  svg_x: 0,  // placeholder
  svg_y: 0,  // placeholder
}));

svg_x/svg_y at (0,0) for every point means the SMOOTH trace will have broken cursor highlighting. The safe options are either (a) remove SMOOTH from the supported type mapping until real SVG coordinates can be derived from the AnyChart DOM, or (b) emit a clear warning that highlighting is unavailable and document it prominently.

---

Performance

6. mapSeriesType recreates its mapping object on every call

export function mapSeriesType(anyChartType: string): TraceType | null {
  const normalized = ...;
  const mapping: Record<string, TraceType> = { ... }; // new object every call

Move mapping to module scope as a const. This is a minor but straightforward fix.

7. Heatmap building is O(n²) due to indexOf inside a loop

const xi = xLabels.indexOf(asString(r.x));   // O(n) per row
const yi = yLabels.indexOf(asString(r.y ...)); // O(n) per row

xSet and ySet are already built above — just use Maps instead:

const xIndex = new Map(xLabels.map((v, i) => [v, i]));
const yIndex = new Map(yLabels.map((v, i) => [v, i]));
const xi = xIndex.get(asString(r.x)) ?? -1;

---

Design / Architecture

8. Event listener re-reads the attribute instead of using event.detail

In src/index.tsx:

document.addEventListener('maidr:bindchart', ((event: CustomEvent<Maidr>) => {
  const target = event.target;
  if (!(target instanceof HTMLElement)) return;
  const json = target.getAttribute(Constant.MAIDR_DATA); // redundant read + re-parse
  if (json) { parseAndInit(target, json, 'maidr-data'); }
}));

The adapter already serialized the object into event.detail. Re-reading and re-parsing the attribute is an unnecessary round-trip. At minimum use JSON.stringify(event.detail) from the already-deserialized detail, or better yet expose a path in parseAndInit that accepts a Maidr object directly.

9. extractAxisTitle uses a fragile .call() pattern

const axisAccessor = axis === 'x' ? chart.xAxis : chart.yAxis;
if (!axisAccessor) return undefined;
const axisObj = axisAccessor.call(chart, 0); // verbose and fragile

Given that xAxis/yAxis are typed as optional functions, the idiomatic call is:

const axisObj = axis === 'x' ? chart.xAxis?.(0) : chart.yAxis?.(0);

10. AnyChartSeries.id() is defined in the interface but never used

The adapter always uses the iteration index as the layer ID. Either remove id: () => string | number from AnyChartSeries or use it to allow stable cross-render layer references.

---

Test Coverage Gaps

• resolveContainerElement string-id path — only null and throw cases are tested. The document.getElementById path (most common real-world usage) is not covered.
• extractAxisTitle — no test at all.
• buildSmoothLayer / buildSegmentedLayer — no tests for these builders.
• The permanent maidr:bindchart listener in src/index.tsx — no integration test verifies that the listener actually calls parseAndInit.
• Histogram test — the existing test is mislabeled; it tests column → BAR, not histogram behavior. Either fix it to exercise the histogram builder directly or remove it to avoid misleading coverage metrics.

---

Nits

• Spelling inconsistency: initialisation (British) vs. initialization (American) — the rest of the codebase uses the American spelling.
• Version bump (3.50.0 → 3.51.0): Confirm this follows the project's release process.
• CDN URL in example: AnyChart requires a paid commercial licence. The comment mentions this in passing, but it should be called out more prominently in any accompanying docs.

---

Summary

Category	Count
Bugs / Correctness	5
Performance	2
Design	3
Test gaps	5
Nits	3

The adapter is well-documented and the overall approach is sound. Most important to address before merging: #1 (dead histogram code with misleading test), #2 (silent default fallback), #3 (unguarded JSON.parse), and #5 (smooth trace broken highlighting). The performance items (#6, #7) are straightforward wins.