Repository navigation
feat(s2): legend navigation, keyboard right-click and hardening for the data navigator - #1097
Open
c-lamoureux wants to merge 65 commits into
Open
c-lamoureux wants to merge 65 commits into
c-lamoureux wants to merge 65 commits into
Conversation
Replace the backtracking-prone [A-Z]{2,}(?=[A-Z][a-z]+\d*|\b) acronym
match with a single negative lookahead ([A-Z]+(?![a-z])), which decides
the split point without re-scanning candidate lengths. This removes the
O(n^2) worst case triggered by long uppercase runs (e.g. name props)
followed by a non-letter character.
SonarCloud: https://sonarcloud.io/project/issues?open=AZ89i0vkqenAEyA5_fKB&id=adobe_react-spectrum-charts
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
/.*(Trendline|MetricRange).*highlightedData$/ has two unbounded wildcards around an alternation, letting an adversarial data source name (many repeated Trendline/MetricRange substrings) force O(n^2) backtracking. The check is really just "contains one of these substrings and ends with this suffix," which includes()/endsWith() express directly and in guaranteed linear time. Applied to both the s1 and s2 legendSpecBuilder, which had the identical regex. SonarCloud: https://sonarcloud.io/project/issues?open=AZ89iz6oqenAEyA5_fJ_&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Prefer the Number-namespaced static method over the global function per typescript:S7773. Fixes the flagged instance in vega-spec-builder-s2/trendlineUtils.ts and the same pattern everywhere else it appeared in the codebase (both trendlineUtils.ts mirrors, both Chart.test.tsx stories files, and the sonar-fork scripts). SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHaHID6Ha28pDOmS&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extract the open- and close-popover branches of handleOpenChange into their own nested functions (openPopoverView/closePopoverView), the same pattern already used for the close branch's clearSelection. Sonar scores nested function bodies separately from their enclosing function, so this drops handleOpenChange's own complexity from 16 to 3 without changing behavior — the open branch's trailing setSelectedSignals/run call, previously a second top-level `if (open)` after the if/else, is folded into openPopoverView since it only ever ran when open was true. SonarCloud: https://sonarcloud.io/project/issues?open=AaDTnyTRvbuDdREVfOEV&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Split the function into the three independent steps it was doing: applyWithinGroupNavigationRules (rebind same-column edges onto the group-local axis), buildColumnsBySeries (map each column's series values to their segment ids), and wireAdjacentColumnNavigation (connect adjacent columns). This is a pure extraction — no behavior change — and drops the parent function from complexity 21 to 2, with each extracted function well under the 15 limit on its own. SonarCloud: https://sonarcloud.io/project/issues?open=AaCnMno1EnEdGe7Q_SMX&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Confirmed by running eslint directly on the file with the blanket disable removed: the only rule that actually fires is @typescript-eslint/ban-ts-comment, triggered by the file's own @ts-nocheck line. Naming it instead of disabling everything keeps any other rule active for this file (it's checked into the repo even though it's generated, since there's no in-repo generator script to re-run). SonarCloud: https://sonarcloud.io/project/issues?open=AaDTnyO8vbuDdREVfOEQ&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Confirmed by running eslint directly on the file with the blanket disable removed: no rule fires here at all, so the comment was pure dead weight rather than suppressing anything. Unlike the sibling en-US.ts (which needs @typescript-eslint/ban-ts-comment for its own @ts-nocheck line), this file has no such comment to guard against. SonarCloud: https://sonarcloud.io/project/issues?open=AaDTnyPTvbuDdREVfOER&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
titleAt (in fieldLabels) and titleAtPosition (in navMetricTitleBySeries) were byte-for-byte identical implementations, just with narrower vs wider position parameter types. Extracted the shared logic into a single module-level getAxisTitleAtPosition and had both useMemo callbacks call it instead. Pure extraction, no behavior change. SonarCloud: https://sonarcloud.io/project/issues?open=AaDTnyTRvbuDdREVfOES&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
showTooltipForFocusedNode and guardHoverParityAgainstMouseClear each took 9 positional parameters, 6 of which (markName, dimension, color, metric, fieldLabels, hasChartInspect) were the same "chart context" shared between them. Bundled those into a FocusTooltipContext object (mirroring the BarHoverParityOptions pattern already used by applyHoverParitySignals in this file), dropping both functions to 4 params. guardHoverParityAgainstMouseClear now forwards its own context straight through to showTooltipForFocusedNode instead of re-listing the fields. Pure refactor, no behavior change. SonarCloud: https://sonarcloud.io/project/issues?open=AaCnMnrAEnEdGe7Q_SMY&id=adobe_react-spectrum-charts https://sonarcloud.io/project/issues?open=AaCnMnrAEnEdGe7Q_SMZ&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- seriesOrder[seriesOrder.length - 1] -> seriesOrder.at(-1): simpler
index access, same result (seriesOrder is already guarded non-empty
above).
- locale prop can be a plain locale-code string OR a Vega Locale
object ({ number?, time? }) — String(locale) would silently produce
"[object Object]" for the object form instead of a usable BCP-47
code. Navigator's own locale prop is string-only, so forward it
only when it actually is a string and let Navigator's default apply
otherwise.
SonarCloud:
https://sonarcloud.io/project/issues?open=AaDTnyTRvbuDdREVfOET&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AaDTnyTRvbuDdREVfOEU&id=adobe_react-spectrum-charts
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces JSON.parse(JSON.stringify(...)) with structuredClone for deep-cloning both the incoming chart data and the Vega spec before Vega mutates them in place. structuredClone is faster and correctly preserves types the JSON round-trip would degrade (e.g. Date fields in time-series data becoming strings). Fixed in both the s2 and s1 VegaChart.tsx, which had the identical pattern at both call sites. SonarCloud (s2 data clone, the linked issue): https://sonarcloud.io/project/issues?open=AZx3iH2jID6Ha28pDOms&id=adobe_react-spectrum-charts Same rule (typescript:S7784) also flagged on the sibling spec clone in this file and on both call sites in the s1 mirror; fixed all four together rather than one at a time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…l chain - buildNodeLabel/getNodeIdForDatum treat dimensionValue as unknown (it comes off an arbitrary data row), so String(dimensionValue) or template interpolation could silently produce "[object Object]" if a caller's dimension field ever held an object. Bail out the same way the existing null check already does, since an object isn't a usable dimension value either way. - metricSeriesLabel && key === metricSeriesLabel.metric -> key === metricSeriesLabel?.metric: same short-circuit behavior (key is always a string, never undefined, so the equality can only be true when metricSeriesLabel is defined — TS narrows it correctly inside the block). SonarCloud: https://sonarcloud.io/project/issues?open=AaDTnyLFvbuDdREVfOEL&id=adobe_react-spectrum-charts https://sonarcloud.io/project/issues?open=AaDTnyLFvbuDdREVfOEN&id=adobe_react-spectrum-charts https://sonarcloud.io/project/issues?open=AaDTnyLFvbuDdREVfOEO&id=adobe_react-spectrum-charts https://sonarcloud.io/project/issues?open=AaDTnyLFvbuDdREVfOEP&id=adobe_react-spectrum-charts https://sonarcloud.io/project/issues?open=AaCnMnr_EnEdGe7Q_SMa&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both catch blocks swallowed the underlying error entirely, rejecting with a generic message and losing whatever fetch/blob failure actually happened. Attach the original error as the new Error's cause so it's still inspectable (e.g. in DevTools or error reporting) without changing the rejection message callers already depend on. Fixed in both s1 and s2, which had the identical pattern. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iH0PID6Ha28pDOmo&id=adobe_react-spectrum-charts https://sonarcloud.io/project/issues?open=AZx3iH0PID6Ha28pDOmp&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Width was number | 'auto' | string — the bare string made 'auto'
redundant (already a string) and let any string through the type
system even though useChartWidth only ever accepts a number, 'auto',
or a percentage string like '75%'. Narrowed it to number | 'auto' |
`${number}%`, mirroring how Height is already number | `${number}%`.
useChartWidth now imports Width instead of redeclaring its own looser
inline union, matching the pattern useChartHeight already follows.
Fixed in both s1 and s2 (identical Width definition and hook in both).
SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHzRID6Ha28pDOmm&id=adobe_react-spectrum-charts
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same JSON.parse(JSON.stringify(...)) -> structuredClone swap as the VegaChart fixes. Fixed in both s1 and s2, which had the identical line. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHz2ID6Ha28pDOmn&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pointMark && pointMark.items.length === 1 -> pointMark?.items.length === 1: same short-circuit result (undefined === 1 is false), more concise. Fixed in both s1 and s2, which had the identical line. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iH1-ID6Ha28pDOmr&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Array#push accepts multiple arguments; two back-to-back push() calls on the same transform array collapse into one call with both items, same insertion order. Fixed in both s1 and s2. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHa7ID6Ha28pDOmV&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same Array#push consolidation as barSpecBuilder. Fixed in both s1 and s2. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHeJID6Ha28pDOmc&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
buildSpec already defines safeClone (structuredClone with a JSON.parse(JSON.stringify(...)) fallback for environments without it) and calls it on the way out. The earlier "copy so we don't mutate the original" step duplicated the same JSON round-trip logic instead of calling safeClone — consolidated onto the one helper. Left safeClone's own internal fallback branch alone: it's an intentional feature-detection fallback for runtimes without structuredClone, not a naive oversight, so replacing it would remove that support rather than fix a lint nit. SonarCloud (the mid-function clone this fixes): https://sonarcloud.io/project/issues?open=AZx3iHdqID6Ha28pDOma&id=adobe_react-spectrum-charts A second issue was reported on the safeClone fallback itself: https://sonarcloud.io/project/issues?open=AZx3iHdqID6Ha28pDOmb&id=adobe_react-spectrum-charts Left as-is per the reasoning above — happy to revisit if that judgment call is wrong. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The doubled backslashes in "\\u00a0\\u00a0" exist so the resulting Vega expression string contains a literal escape for Vega's own expression parser to interpret at signal-evaluation time, not a JS-level unicode escape. String.raw expresses that directly with single backslashes instead of doubling them. Verified byte-identical output and confirmed against the existing test asserting this exact signal string. Fixed in both s1 and s2, which had the identical line. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHbiID6Ha28pDOmW&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BarAnnotationOptions.style has been marked @deprecated since before the s1/s2 split, but style.width is still the only way to override an annotation's computed width and remains part of the public BarAnnotationProps type — no replacement field was ever added. Rather than design a new public width prop as a side effect of a lint cleanup, document why the deprecated field is read here at both call sites, in both s1 and s2. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHakID6Ha28pDOmT&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
transforms.findIndex(...) === -1 is an existence test; .some() says that directly instead of comparing an index to a magic -1. Fixed in both s1 and s2, which had the identical line. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHfyID6Ha28pDOmi&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Vega's ScaleField type is string | SignalRef, so scale.domain.fields entries aren't guaranteed to be plain field names -- a SignalRef would silently stringify to "[object Object]" via .toString(). Added an isFieldName type guard and used it everywhere a domain field was being coerced to a string (getFacets, getFacetsFromKeys, scaleHasKey), skipping a scale's facet entirely when its field isn't a plain name rather than producing a broken facet id. This library's own scale construction (addDomainFields) only ever pushes plain strings, so this is defensive rather than a fix for an observed bug -- but the type doesn't guarantee it, and the existing `as string` cast at two of these call sites was already papering over exactly this gap. Fixed identically in both s1 and s2. SonarCloud (the linked issue): https://sonarcloud.io/project/issues?open=AZx3iHYpID6Ha28pDOmJ&id=adobe_react-spectrum-charts Same rule (typescript:S6551) was also open on 6 other spots in this file (lines 65, 90 x2, 95 x2, 110); fixed all of them together. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
SpectrumColor | string, NumberFormat's preset union, and
ChartSymbolShape's union all had a bare `| string` appended after a
curated set of named literals, meant to give editor autocomplete for
the common values while still accepting any raw string (a hex color,
an Intl.NumberFormat-style string, etc). TypeScript silently collapses
`Literal | string` to plain `string` for completion purposes, so the
literals were dead weight — which is what Sonar's rule flags.
Replaced `| string` with `| (string & NonNullable<unknown>)`, the
standard "LiteralUnion" idiom: it keeps the literal members
distinguishable from a plain string so editors still suggest them,
while remaining assignable from/to any string at the type level. Used
NonNullable<unknown> rather than the more common `(string & {})`
because this repo's `@typescript-eslint/ban-types` rule bans bare `{}`
as a type and suggests that exact replacement for "any non-nullish
value" in its own error message. No runtime behavior change anywhere.
Fixed at all 19 sites across s1 and s2: SpectrumColor/SpectrumVizColor
color props (axisAnnotation, metricRange, scatterPath, trendline,
referenceLine (s1 only, no s2 equivalent), ColorFacet), plus
NumberFormat and ChartSymbolShape in specUtil.types.ts.
SonarCloud:
https://sonarcloud.io/project/issues?open=AZx3iHXFID6Ha28pDOl9&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHXFID6Ha28pDOl-&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHUMID6Ha28pDOl5&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHVGID6Ha28pDOl6&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHVsID6Ha28pDOl7&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHX2ID6Ha28pDOmD&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHX2ID6Ha28pDOmE&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHX2ID6Ha28pDOmF&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHX2ID6Ha28pDOmG&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHX2ID6Ha28pDOmH&id=adobe_react-spectrum-charts
https://sonarcloud.io/project/issues?open=AZx3iHX2ID6Ha28pDOmI&id=adobe_react-spectrum-charts
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The comment "Remove undefined values" already stated the intent, but Sonar can't tell that apart from a naive clone -- and structuredClone would actually change behavior here: it keeps undefined-valued keys (just with an undefined value) instead of dropping them entirely like JSON.stringify does, which Vega's encode entries need. Documented why the JSON round-trip is the correct choice at this call site rather than swapping it. Fixed in both s1 and s2. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHZZID6Ha28pDOmR&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ication Same underlying issue as the earlier legendFacetUtils fix: Vega's ScaleField type is string | SignalRef, so scale.domain.field(s) isn't guaranteed to be a plain string. getScaleField (scaleUtils.ts) and getFacetsFromScales (specUtils.ts) both called .toString() on it directly; now they only use it when it's actually a string, returning undefined/skipping otherwise instead of risking "[object Object]". Also narrowed getPathFromIcon's `Icon | string` parameter to `Icon | (string & NonNullable<unknown>)`, the same literal-union-preserving fix as the previous commit, so the named icon literals stay autocomplete-visible instead of collapsing into plain string. Fixed in both s1 and s2, which had identical code in all three spots. SonarCloud: https://sonarcloud.io/project/issues?open=AZx3iHfPID6Ha28pDOmg&id=adobe_react-spectrum-charts https://sonarcloud.io/project/issues?open=AZx3iHfPID6Ha28pDOmh&id=adobe_react-spectrum-charts https://sonarcloud.io/project/issues?open=AZx3iHexID6Ha28pDOmd&id=adobe_react-spectrum-charts https://sonarcloud.io/project/issues?open=AZx3iHexID6Ha28pDOme&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same Array#push consolidation as the earlier barSpecBuilder/ bulletMarkUtils fixes. The s2 version already does this (and combines even more sources into the one call) -- s1 had fallen behind. SonarCloud: https://sonarcloud.io/project/issues?open=AZ9NqwwVHjdVvPVL6RiS&id=adobe_react-spectrum-charts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lone" This reverts commit 4216f3d. That commit swapped buildSpec's mid-function and final spec clones from JSON.parse(JSON.stringify(...)) to structuredClone (via a safeClone helper), to resolve a SonarCloud maintainability nit. It passed every targeted test run at the time, but broke `S2Line.test.tsx`'s "lines should be the correct color" test (stroke-width ends up missing entirely) — visible only when running the full suite, never in isolation or small groups, which is why it slipped through repeated verification during that session. Confirmed via git bisect against main (full-suite runs, not isolated single-file runs — isolated runs pass at every commit, good or bad, which is what made the original verification misleading): main passes 292/292 cleanily, and 4216f3d is the first commit where the full suite drops to 291/292 with this exact failure. Tried using structuredClone directly (skipping the safeClone wrapper) as an alternative fix; that still failed, confirming the meaningful difference is JSON.stringify's behavior itself (e.g. dropping undefined-valued keys, which structuredClone preserves) rather than anything about the wrapper function. Reverting restores 292/292. The underlying SonarCloud finding this reintroduces is a minor "prefer structuredClone" style nit (https://sonarcloud.io/project/issues?open=AZx3iHdqID6Ha28pDOma&id=adobe_react-spectrum-charts), not a functional requirement — not worth re-attempting without a much narrower, verified-safe change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
S1 (react-spectrum-charts, vega-spec-builder) is in maintenance mode, so functional changes made only to satisfy SonarCloud are not applied there. Restores S1 to main and suppresses each existing S1 finding with a line-level NOSONAR comment, guarded by prettier-ignore where formatting would move the comment off the reported line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rency regex Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix: SonarCloud follow-up for #937 (S1 NOSONAR, S2 fixes)
…s, and number/time dimensions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix: S2 bar navigation for multi-field series, dodged-and-stacked bars, and number/time dimensions
Covers entry/exit, drilling, arrows, Escape tooltip dismissal, popover and onClick activation, click-to-focus, the bottom axis region, and focus across host updates (kept on no-op/signal updates, reset on rebuilds; a data update keeping focus is marked as a known failure). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # packages/react-spectrum-charts-s2/src/RscChart.tsx # packages/react-spectrum-charts-s2/src/VegaChart.tsx # packages/react-spectrum-charts-s2/src/hooks/useChartImperativeHandle.tsx # packages/vega-spec-builder-s2/src/bar/barFocusRingUtils.test.ts # packages/vega-spec-builder-s2/src/bar/barFocusRingUtils.ts # packages/vega-spec-builder-s2/src/bar/barSpecBuilder.ts # packages/vega-spec-builder-s2/src/chartSpecBuilder.ts # packages/vega-spec-builder-s2/src/specUtils.ts # packages/vega-spec-builder-s2/src/types/marks/supplemental/metricRangeSpec.types.ts # packages/vega-spec-builder-s2/src/types/marks/supplemental/trendlineSpec.types.ts
test(s2): pin accessible navigation behavior through Chart
…roller - navigationSession (was dataNavigatorAdapter) mounts for one view and returns destroy/setActions/focusEntry instead of tracking listeners in module WeakMaps - navigationController remounts only when the view or navigation structure changes, after the view's pending run; action changes update in place - useDataNavigator replaces the Navigator component and RscChart's derivation - a data update now resets focus to the entry button - dataNavigator/ may not import React (eslint) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ategies The session now owns only the shared work: DOM, focus, keys, entry/exit and teardown. Chart content and the x-axis are standalone NavigationRegions; chart-type specifics live behind ChartNavStrategy (bar today, line to follow). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The spec builder records navigable legends in usermeta; the session adds a legend region whose items mirror chart content's leaves. Legend clicks and hovers are dispatched through a per-view event registry, and the controller restores focus to the same node across rebuilds (including hiddenSeries toggles). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d legend series Dispatches a synthetic contextmenu at the focused mark's center through the view's own listeners, so rightClick popovers and onContextMenu run unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ed accessibleNavigation module Bar keeps its own rings but uses the shared ring stroke, offsets and navigation key helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…loud Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ity guide Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-bar marks, trellised bars, and charts with more than one mark now get no focus rings, legend navigation, or navigation config, and a console warning says why. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lopment After building the navigation, development builds compare its leaves with the bar mark's rendered items and warn about leaves with no bar and bars no leaf reaches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tion Renders each bar dashboard variation with every dataset and accessibleNavigation on, and requires it to navigate in step with the rendered bars, or to be listed as unsupported with a reason. The dashboard gains a "Keyboard navigation" switch. Fixes the crash this found: hiding every series left the chart content without an entry point. Navigation now starts at the legend so series can be shown again. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c-lamoureux
changed the title
Wraps unionBounds in the reduce callback so it doesn't receive the index and array, and drops a redundant `| string` from NumberFormat parameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c-lamoureux
marked this pull request as ready for review
November 10, 2026 20:28
Adopt the shared S2 message catalog from main: navigation strings (including the legend descriptions) now live in intl/en-US.json, the navigator reads the Provider locale, and the region, entry button and exit note are localized. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Contributor
|
🎨 Storybook -> https://opensource.adobe.com/react-spectrum-charts/PR-1097/ 🎨 S2 Storybook -> https://opensource.adobe.com/react-spectrum-charts/PR-1097-s2/ 🎨 S2 Dev Storybook -> https://opensource.adobe.com/react-spectrum-charts/PR-1097-s2-dev/ 📚 Docs -> https://opensource.adobe.com/react-spectrum-charts/PR-1097-docs/ |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merges
dataNavigator2(everything ondataNavigator, plus the review stack built on it) intomain.mainalready has the S2 bar keyboard navigator; this extends it, hardens it, and restructures it so more chart types can follow.dataNavigatorstays the target for the line navigator PR, which will be reconciled after this lands.Summary
Bar navigation fixes
colorarray) are navigable, with bar and group focus rings.colorandopacity) are distinct nodes and read every field.0and empty-string dimension values no longer break navigation;dimensionDataType="time"with date strings works.ordernavigate in drawn order, not reversed.onClickand open the popover, and focus drives hover. The bottom-axis region is added for the defaultdimension.Legend navigation and keyboard right-click
accessibleNavigationon, the legend is its own region, reached with the arrow pointing at it. Enter drills into series, then into that series' marks; arrows follow the rendered rows and columns.onClick, or toggle). Toggled-off series are skipped by content navigation, and focus is restored after a toggle with the new state announced.onMouseOver/onMouseOutfire on focus so controlled legends follow.rightClickpopovers on marks and legend series and callsonContextMenu.Architecture
accessibleNavigationmodule, and records the navigation config inusermeta.navigation, so the hook no longer rebuilds it from React children.Production hardening
SonarCloud maintainability (S2)
localebreaking accessible color names; scale/domain fields stringified as[object Object].widthnarrowed tonumber | 'auto' | \${number}%``.jsonClone(in@spectrum-charts/core-s2/utils) andstructuredClone.NOSONAR.Docs and tooling
s2-data-navigatoragent skill.lint:sonarhonorsNOSONARandsonar.exclusionslike SonarCloud.Consolidates the review stack #1084–#1093 and supersedes #940.