Skip to content

Conversation

@Mgetz10
Copy link
Collaborator

@Mgetz10 Mgetz10 commented Nov 25, 2025

Summary

Brush selector revival!

Testing Steps

  1. Open a linear time based chart in the editor
  2. Select the "Brush slider" option in date/category axis
  3. test keyboard navigation and "show X dates from end" option as well

Optional

Storybook Links

http://localhost:6006/?path=/story/components-templates-chart-brushslider--brush-slider-enabled

Screenshots

Screenshot 2025-12-03 at 11 44 28 AM

@Mgetz10 Mgetz10 requested a review from jayb November 25, 2025 22:47
@jayb jayb added the Hold label Nov 26, 2025
@Mgetz10 Mgetz10 requested a review from Copilot December 3, 2025 16:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR revives the brush slider functionality for linear time-based charts, replacing the previous D3-based implementation with a new visx-based component that includes improved accessibility features and keyboard navigation support.

Key Changes:

  • Complete rewrite of brush selector component using visx library
  • Added keyboard accessibility with arrow key navigation
  • Implemented configurable default selection (by count or percentage)
  • Added duplicate tick prevention logic across multiple chart rendering functions

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/chart/src/components/Brush/BrushSelector.tsx New comprehensive brush selector implementation with accessibility features
packages/chart/src/components/LinearChart.tsx Integrated new brush selector as independent overlay with positioning logic
packages/chart/src/components/EditorPanel/EditorPanel.tsx Added brush configuration controls including default date count option
packages/chart/src/hooks/useScales.ts Enhanced tick calculation to prevent duplicates when data is filtered
packages/chart/src/helpers/filterAndShiftLinearDateTicks.ts Added duplicate tick removal logic
packages/chart/src/helpers/countNumOfTicks.ts Added cap on tick count based on data length
packages/chart/src/data/initial-state.js Added brush configuration defaults
packages/chart/src/_stories/ChartBrush.stories.tsx New storybook examples for brush functionality
packages/chart/src/_stories/ChartBrush.Editor.stories.tsx Comprehensive editor tests for brush features
Comments suppressed due to low confidence (1)

packages/chart/src/components/Brush/BrushSelector.tsx:1

  • This date/number/string conversion pattern appears multiple times in this file (lines 60, 1492). Consider extracting it into a helper function getTickValueKey(value) to reduce duplication and improve maintainability.
import React, { FC, useContext, useMemo, memo, useRef, useEffect, useState, useCallback } from 'react'

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Mgetz10 Mgetz10 force-pushed the enhancement/dev-11787-brush branch from 1140a7f to 77b138a Compare December 3, 2025 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants