Skip to content

feat: chart interaction - #500

Draft
hcopp wants to merge 55 commits into
masterfrom
hunter/chart-interaction-2
Draft

feat: chart interaction#500
hcopp wants to merge 55 commits into
masterfrom
hunter/chart-interaction-2

Conversation

@hcopp

@hcopp hcopp commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

What changed? Why?

This PR

  • Changes our mental model for chart interaction from 'scrubbing' to 'highlighting'
    • This matches material ui, I also thought about other terms since we aren't always highlighting but this makes sense with 'highlight state' instead of interaction state
    • This now supports multi touch natively, whereas scrubber was only 1
      • We thought about supporting a toggle for single/multi touch but this but it would make typecheck funky
      • The downside with this is that simple logic for customers will now need to grab the first touch rather than getting the value directly
    • Customers can now control the state, which enables interaction between other aspects of the interface
  • We still are opt in, so users need enableHighlighting
  • All this old logic is deprecated, not removed

The PR has a lot of changes for migrating our examples from scrubbing to highlighting, the core logic shift is in CartesianChart and HighlightProvider

  • We use accessibilityLabel at root level to determine VoiceOver announcements when interacting

Types of changes you will see

  • Renaming 'scrubber' to 'highlighting' such as enableScrubbing -> enableHighlighting
  • Adding fadeOnHighlight support to bar

Root cause (required for bugfixes)

UI changes

Web

In this example, we render the scrubber beacon at a specific spot when the user wasn't interacting:
image

In this example, the user interacting with one chart highlights the corresponding date on the other chart.
image

Highlight by series
image

Horizontal bar chart interaction
image

Full customization
image

Mobile

Testing

How has it been tested?

  • Unit tests
  • Interaction tests
  • Pseudo State tests
  • Manual - Web
  • Manual - Android (Emulator / Device)
  • Manual - iOS (Emulator / Device)

Testing instructions

Illustrations/Icons Checklist

Required if this PR changes files under packages/illustrations/** or packages/icons/**

  • verified visreg changes with Terran (include link to visreg run/approval)
  • all illustration/icons names have been reviewed by Dom and/or Terran

Change management

type=routine
risk=low
impact=sev5

automerge=false

@cb-heimdall

cb-heimdall commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/2
Emergency override progress: 0/5
These approvals do not satisfy normal or CODEOWNER requirements because the commit identity is unverified.
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 1
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2
CODEOWNERS 🟡 See below

🟡 CODEOWNERS

Code Owner Status Calculation
ui-systems-eng-team 🟡 0/1
Denominator calculation
Additional CODEOWNERS Requirement
Show calculation
Sum 0
0
From CODEOWNERS 1
Sum 1

@hcopp hcopp changed the title Hunter/chart interaction 2 feat: chart interaction Apr 21, 2026
@@ -20,6 +20,7 @@ const BRIDGED_CONTEXTS: React.Context<any>[] = [
ThemeContext,
CartesianChartContext,
ScrubberContext,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need to keep ScrubberContext in for now.

*/
highlightScope?: HighlightScope;
/**
* @deprecated Use `enableHighlighting={false}` instead. This will be removed in a future major release.

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.

should it be enableHighlighting={true}

accessibilityLabel={accessibilityLabel}
accessibilityLabel={
typeof accessibilityLabel === 'string' ? accessibilityLabel : undefined
}

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.

what types can accessibilityLabel be other than string

@@ -628,7 +689,9 @@ export const CartesianChart = memo(
) : (

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.

nit: the nodes in both ternary conditions look very similar, maybe some of them can be memoed and reused

Co-authored-by: Cursor <cursoragent@cursor.com>
| 'onHighlightChange'
| 'highlight'
| 'highlightScope'
| 'fadeOnHighlight'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We omit scrubbing props so I'm adding highlighting props in the same manner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants