feat: add Highcharts charting library adapter

[jooyoungseo]

Add a new maidr/highcharts entry point that converts rendered Highcharts
chart instances into MAIDR-compatible data structures. This enables
accessible non-visual access (audio sonification, text descriptions,
braille output, keyboard navigation) for Highcharts visualizations.

Supported chart types:

• Bar / Column (with orientation detection and inverted charts)
• Line / Spline / Area / Areaspline (multi-line support)
• Scatter
• Boxplot
• Heatmap
• Histogram

Key components:

• adapter.ts: Core highchartsToMaidr() conversion function
• selectors.ts: CSS selector generation scoped per chart container
• sync.ts: createHighchartsSync() for programmatic tooltip and
  point highlighting (bidirectional visual synchronization)
• types.ts: Minimal Highcharts type definitions (no runtime dependency)

Also includes a Vite build config, package.json exports for
maidr/highcharts, and an example HTML page demonstrating integration.

Closes #539

https://claude.ai/code/session_01UYVmQa6fa2p7vC8LrbbYpX

[claude]

PR Review: feat: add Highcharts charting library adapter

This is a well-structured addition that fills an important gap — enabling MAIDR accessibility features for Highcharts visualizations. The overall design is clean and the type definitions are sensibly minimal. Here are my findings:

---

Bugs / Correctness Issues

1. Null safety in convertHeatmapSeries (adapter.ts ~L295)

for (const p of series.data) {
  if (p.y !== null) {
    const xIdx = Math.round(p.x);
    const yIdx = Math.round(p.y);  // p.y typed as number | null

TypeScript still considers p.y as number | null inside this block without a type assertion. More critically, for heatmaps, p.y holds the y-axis index — using it as a grid index assumes Highcharts heatmaps always use integer index values starting at 0. This breaks when the heatmap has numeric (non-category) axes. A more robust approach would be to resolve the index via yCategories.indexOf(...) or clamp/validate the rounded index against bounds.

2. Unreliable histogram bin width fallback (adapter.ts ~L350)

const x2 = typeof opts.x2 === 'number' ? opts.x2 : p.x + 1;

Using p.x + 1 assumes every bin has width 1. Real histograms can have variable-width bins. A safer fallback would be to look at the next point's x value, or read from the series options if available.

3. filterSeries silently drops explicitly-requested series

function filterSeries(chart, indices?) {
  const all = indices ? indices.map(i => chart.series[i]).filter(Boolean) : chart.series;
  return all.filter(s => s.visible);  // silently discards invisible series
}

If a caller passes seriesIndices: [2] but series 2 is hidden, it is dropped without any warning. The intent of explicitly passing indices is likely to override the visibility filter. Consider warning or skipping the visibility filter when explicit indices are provided.

4. Wrong fallback value in heatmap cell (adapter.ts ~L302)

const value = p.options?.value;
points[yIdx][xIdx] = typeof value === 'number' ? value : (p.y as number);

p.options is typed as Record<string, unknown>, so value is unknown. The typeof guard is correct, but the fallback to p.y is wrong — p.y is the y-axis index for heatmaps, not the cell value. This would produce incorrect accessibility output (reporting a grid coordinate as a heat value). The fallback should be 0 or should log a warning.

---

Architecture / Design Concerns

5. HighchartsSync has no actual MAIDR integration

The sync.ts module exposes highlightPoint(seriesIndex, pointIndex) but there is no mechanism to wire this to MAIDR's navigation events. The JSDoc says "Attach to a MutationObserver or MAIDR's navigation events" but MAIDR's event system isn't publicly exposed in a way that makes this straightforward.

Without a clear integration path, this API may go unused. Consider documenting a concrete wiring example, or adding a navigation event hook in MAIDR's Observer flow that external adapters can subscribe to.

6. Multi-line layer loses series metadata

return {
  id: String(first.index),         // only first series index
  title: first.name || undefined,   // only first series name
  ...

When multiple line series are merged into a single MaidrLayer, the ID and title reflect only the first series. This could be confusing when inspecting the output. Consider using a combined ID (e.g. "line-0-1-2") or leaving it undefined.

7. Legend count can mismatch layer count

if (seriesToConvert.length > 1) {
  subplot.legend = seriesToConvert.map(s => s.name);
}

seriesToConvert includes all series, but layers may have fewer entries (N line series → 1 combined layer + M other layers). Depending on how MAIDR uses subplot.legend, the array length mismatch could cause issues.

---

Code Quality

8. window._sync debug assignment left in example (highcharts-barplot.html:48)

window._sync = sync; // Expose for debugging.

This should be removed before merging.

9. Re-initialization step missing in example

The example sets maidr-data after MAIDR's script has already run (DOMContentLoaded fires, MAIDR scans the DOM, then the attribute is set). The chart will not be made accessible as written — step 4 in the comments is intentionally left blank but no actual re-init call is provided. This either needs a working re-init call or the example needs to be restructured so the attribute is present before MAIDR initializes.

10. Module-level counters (chartCounter, selectorCounter)

Both counters work fine at runtime, but in test environments where modules may be reset they could produce non-deterministic IDs. Consider exporting a reset function for testing, or seeding via the options object.

---

Missing Tests

There are no unit or integration tests for any of the new adapter code. Given the data transformation logic (especially heatmap grid construction and histogram bin extraction), test coverage is important. At minimum, tests for:

• highchartsToMaidr output shape for each supported chart type
• convertHeatmapSeries grid indexing correctness
• filterSeries with explicit indices including hidden series
• selector output format for each chart type

---

Minor Nits

• resolveSeriesType falls back to 'line' for unknown series types. Since line series are handled specially (grouped), an unknown type would silently become a line trace rather than being skipped. Falling back to null (so convertSeries can return null and skip it) would be safer.
• getAxisLabel falls back to 'X' / 'Y' — consider empty string '' to match existing MAIDR conventions, unless 'X'/'Y' is the established default.
• The dispose() in HighchartsSync only calls clearHighlight() with no additional cleanup. Worth a comment noting this is intentional.

---

Summary

Category	Severity	Count
Bugs	Medium	4
Design	Low–Medium	3
Missing tests	Medium	1
Nits/style	Low	4

The core idea and structure are solid. The most important items to address before merging are: the broken example HTML (step 4), the heatmap fallback value bug, the silent filterSeries drop behavior, and the absence of test coverage.

[claude]

PR Review: Highcharts Adapter

Great addition! The overall structure is clean and the use of custom type definitions to avoid a hard dependency on the Highcharts package is the right approach. Here are some findings across quality, correctness, and design:

---

Bugs / Correctness

1. getBoundingClientRect() returns wrong coordinate space for smooth traces (adapter.ts:783–787)

const bbox = graphic.getBoundingClientRect();
svgX = bbox.x + bbox.width / 2;
svgY = bbox.y + bbox.height / 2;

getBoundingClientRect() returns viewport-relative coordinates (affected by scroll position and page layout), not SVG-space coordinates. If the chart is not at the page origin, svg_x / svg_y will be offset incorrectly. The MAIDR smooth trace expects SVG-relative coordinates. Use SVGElement.getCTM() or Highcharts' own point.plotX / point.plotY properties instead.

---

2. Boxplot silently defaults missing statistics to 0 (adapter.ts:580–589)

min: p.low ?? 0,
q1: p.q1 ?? 0,
q2: p.median ?? 0,
q3: p.q3 ?? 0,
max: p.high ?? 0,

Defaulting to 0 when these fields are absent produces silently wrong box summaries. A Highcharts boxplot point without q1/median/q3 indicates bad data — it should either be filtered out (like other converters do with p.y !== null) or emit a console.warn. The current behavior will cause MAIDR to read and announce incorrect statistics.

---

3. lineSelectors and smoothSelectors are identical (selectors.ts:899–903, 955–959)

Both functions return path.highcharts-graph selectors. If SMOOTH traces need a different selector than LINE traces, this is a bug. If they are genuinely the same, consider sharing one function.

---

Design / Architecture

4. convertSmoothSeries is exported but never called by highchartsToMaidr

Spline/areaspline series enter convertLineSeries and get TraceType.LINE. convertSmoothSeries is only reachable if a caller constructs it manually. The PR description lists "spline (smooth) → SMOOTH (when only one series)" as a supported mapping, but highchartsToMaidr never routes there. Either wire it inside the adapter or document clearly that this is a manual-use-only export.

---

5. Stacking mode detection reads only the first series (adapter.ts:347–348)

const first = barSeries[0];
const stacking = getStackingMode(first, chart);

If series have mixed or inconsistent stacking settings, only the first series determines the layout type for all of them. Consider checking that all series agree on the stacking mode (or at least that all visible ones match) and logging a warning if they diverge.

---

6. ensureContainerId mutates the DOM unexpectedly (selectors.ts:874–879)

Assigning target.id as a side effect of a helper called ensureContainerId is surprising. If the container already has id="chart" and the user relies on CSS or other scripts targeting it, this is benign — but if the container has no id, the silently mutated DOM can conflict with surrounding page code. The mutation is necessary, but the behavior should be documented clearly or surfaced as a warning.

---

API Surface

7. Test-only utilities exported as public API

resetChartCounter() and resetSelectorCounter() exist solely for deterministic test output. Exposing them in the public maidr/highcharts entry point leaks internal concerns. Consider moving them to a testing sub-export (e.g., maidr/highcharts/testing) or only exporting them in dev/test builds. Same applies to convertSmoothSeries if it stays as an advanced/manual-use export.

---

Test Coverage

8. No unit tests included

The project has a test suite (npm test). The adapter has significant conversion logic and edge-case handling (heatmap grid inference, stacking detection, orientation resolution, candlestick trend classification) that would benefit from unit tests. At minimum, tests covering each series type converter and the stacking/orientation branches would catch regressions when Highcharts API details change.

---

Minor

• p.y as number casts after a .filter(p => p.y !== null) guard are correct, but TypeScript narrowing should handle this without a cast if the filter's predicate is typed as a type guard ((p): p is ... => p.y !== null).
• volume: ... ?? 0 in the candlestick converter (adapter.ts:741) defaults to 0 rather than undefined. If MAIDR's candlestick rendering treats 0 and "no volume" differently, this may cause incorrect descriptions.
• The example HTML (examples/highcharts-barplot.html) dynamically appends MAIDR's script tag after setting maidr-data. The comment warns about load order — consider adding a note about the potential race condition if the script loads synchronously before the chart finishes rendering.

---

Overall this is solid foundational work. The type definitions, build config, and conversion structure are well-organized. Addressing the coordinate-space bug in smooth traces and the boxplot silent-zero issue would be the highest-priority fixes before merging.

[claude]

Code Review: Highcharts Adapter

Great addition! The overall architecture is clean — a standalone adapter with no runtime Highcharts dependency is exactly the right approach. The separation into adapter.ts, selectors.ts, sync.ts, and types.ts is well-reasoned and follows a sensible module boundary.

Here are my findings, roughly in order of severity:

---

Bugs

1. Module-level counter inconsistency

adapter.ts has chartCounter (no reset export) while selectors.ts has selectorCounter (with resetSelectorCounter() exported for tests). These are separate counters with no relationship to each other — if both IDs need to be consistent across test runs, both should be resettable. As-is, chartCounter silently accumulates across tests.

2. filterSeries ignores explicit visibility intent

When seriesIndices are explicitly provided, the function still skips hidden series with just a warning. If a caller explicitly asks for index 2, they likely want it regardless of Highcharts visibility state. Either honour the explicit request or throw, rather than silently dropping it.

3. Histogram bin width degrades to zero

In convertHistogramSeries, when x2 is absent, binEnd === binStart, so xMin === xMax. A zero-width bin will likely cause rendering/navigation issues. Consider falling back to the next bin start or emitting a warning.

4. Undefined y passes through null filter

Several converters use filter(p => p.y !== null) but p.y is typed as number | null. If Highcharts ever sets p.y = undefined (which it does for missing points in some series types), these points will not be filtered and p.y as number casts will silently produce NaN. The safer guard is p.y != null (covers both null and undefined).

5. Heatmap value fallback silently zero-fills

In convertHeatmapSeries, the fallback path (cellValue ?? 0) silently zero-fills cells whose options.value is absent. If a heatmap series is missing value on every point, the resulting grid is all-zeros with no warning.

---

Code Quality

6. resolveSeriesType called three times per series

In highchartsToMaidr, three separate .filter() calls each invoke resolveSeriesType. Caching the resolved type once per series would avoid the repeated work.

7. barSelector and heatmapSelector are identical

Both generate the same selector pattern (rect.highcharts-point). While the separate names add clarity, this duplication could cause the two to drift over time. A shared rectSelector helper called by both would prevent that.

8. Fragile implementation detail in sync.ts JSDoc

The Option 1 example hardcodes Math.floor(siblings.indexOf(el) / 2) // cloned pairs. This leaks MAIDR's internal highlight implementation into public documentation. If MAIDR's highlight mechanism ever changes, this example breaks silently. Consider replacing with the Redux-based Option 2 approach instead.

9. Legend assignment uses first series title for grouped bar layers

subplot.legend = layers.map(l => l.title ?? `Series ${l.id}`);

For stacked/dodged layers, l.title is the name of the first series only. A stacked chart with series ["Men", "Women"] would produce a legend entry of "Men" rather than representing all contributing series.

---

Test Coverage

No tests are included for any of the new files. The resetSelectorCounter export suggests testing was planned. Before merge, at minimum the following cases should have unit tests:

• Single bar, dodged, stacked, and normalized bar groups
• Multi-line series (data grouping into LinePoint[][])
• Heatmap grid construction from category vs. numeric axes
• filterSeries with both implicit (visible) and explicit index modes
• ensureContainerId with and without a pre-existing id

---

Minor Observations

• The example HTML loads Highcharts from CDN with no SRI hash. For a production-facing example, an integrity attribute would be good practice.
• Only a bar/column example is provided. A multi-chart example covering line, scatter, and heatmap would help adopters validate the integration before edge cases surface.
• convertLineSeries sets fill: series.name || undefined on each LinePoint. This is a valid use of the optional fill field, but a brief comment explaining that fill here distinguishes lines in a multi-series layer would help readers unfamiliar with MAIDR's LinePoint type.

---

Overall this is solid work. The type definitions, JSDoc, and adapter logic are well thought-out. Addressing the null/undefined handling, the histogram bin-width bug, and adding a minimal test suite would make this ready to merge.

[Copilot]

Pull request overview

Adds a new maidr/highcharts adapter entry point that converts rendered Highcharts chart instances into MAIDR-compatible data structures, enabling sonification, text descriptions, braille output, and keyboard navigation for Highcharts visualizations. Includes a small <Maidr> pointer-down focus shim and model-layer support for per-cell heatmap selector grids and path-encoded scatter coordinates.

Changes:

• New src/adapters/highcharts/{adapter,selectors,sync,types,index}.ts providing highchartsToMaidr() and createHighchartsSync(), plus DOM attribute stamping and SVG <path> splitting for box/candlestick highlight subparts.
• Heatmap model now accepts string[][] per-cell selector grids; ScatterTrace falls back to parsing d="M x y …"; MaidrLayer.selectors widened to include string[][]; <Maidr> wrapper gains a click-to-focus handler.
• Build/site/docs wiring: new Vite build target, package.json export, docs page, sitemap entry, ten example HTML files, and template menu link.

Reviewed changes

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

Show a summary per file

File	Description
src/adapters/highcharts/adapter.ts	Core Highcharts→MAIDR conversion with stacking/orientation detection and SVG stamping/splitting.
src/adapters/highcharts/selectors.ts	Per-chart scoped selector generators including 2D heatmap grid and box/candle subpart selectors.
src/adapters/highcharts/sync.ts	createHighchartsSync() hover/tooltip controller.
src/adapters/highcharts/types.ts	Minimal structural Highcharts type definitions (no runtime dep).
src/adapters/highcharts/index.ts	Public re-exports for maidr/highcharts.
src/model/heatmap.ts	Adds string[][] selector grid branch alongside existing string selector.
src/model/scatter.ts	Fallback marker-center extraction from path d attribute.
src/type/grammar.ts	Widens MaidrLayer.selectors to permit string[][].
src/maidr-component.tsx	Pointer-down handler to focus the wrapper when inert SVG children are clicked.
package.json	Adds ./highcharts export.
scripts/build.js	Adds Vite build config for the Highcharts entry.
scripts/build-site.js	Adds docs page, examples list, sitemap, and template active-state for Highcharts.
docs/template.html	Adds Highcharts menu link.
docs/highcharts.md	Full integration guide and API reference.
examples/highcharts-*.html	10 demo pages covering supported chart types.

[xabilitylab]

🎉 This PR is included in version 3.69.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀