Skip to content

feat: add keyboard navigation for select charts #4481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: add keyboard navigation for select charts #4481

wants to merge 1 commit into from

Conversation

@GordonSmith
Copy link
Member

Checklist:

  • The commit message is properly formatted and free of typos.
    • The commit message title makes sense in a changelog, by itself.
    • The commit message includes a "fixes" reference if appropriate.
    • The commit is signed.
  • The change has been fully tested:
    • I have viewed all related gallery items
    • I have viewed all related dermatology items
  • There are no similar instances of the same problem that should be addressed
    • I have addressed them here
    • I have raised new issues to address them separately

Testing:

@GordonSmith GordonSmith requested a review from Copilot October 23, 2025 12:55
Copilot AI reviewed Oct 23, 2025
View reviewed changes
Copy link

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 adds keyboard navigation functionality to Column and Pie chart components, enabling users to navigate and interact with chart elements using the Tab key and Space/Enter keys for selection. The changes also update repository references from "trunk" to "main" branch across multiple package.json files.

Key changes:

  • Adds keyboard navigation support with Tab, Space, and Enter keys for Column and Pie charts
  • Implements ARIA attributes and focus states for improved accessibility
  • Updates CSS styling for hover, focus, and selection states with visual feedback
  • Updates homepage URLs from "trunk" to "main" branch in package.json files

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
packages/util/package.json Updated homepage URL from trunk to main branch
packages/observablehq-compiler/package.json Updated homepage URL from trunk to main branch
packages/markdown-it-plugins/package.json Updated homepage URL from trunk to main branch (with typo)
packages/dataflow/package.json Updated homepage URL from trunk to main branch
packages/chart/src/Pie.ts Added keyboard navigation, ARIA attributes, and slice padding functionality to Pie chart
packages/chart/src/Pie.css Added CSS for focus states, hover effects, and keyboard navigation styling
packages/chart/src/Column.ts Added keyboard navigation, ARIA attributes, and Tabster integration to Column chart
packages/chart/src/Column.css Updated CSS for selected state, added focus and hover styles
packages/chart/index.html Added demo controls for testing keyboard navigation on both chart types

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

packages/markdown-it-plugins/package.json
"url": "https://github.com/hpcc-systems/Visualization/issues"
},
"homepage": "https://github.com/hpcc-systems/Visualization/tree/trunk/packages/markdonw-it-plugins"
"homepage": "https://github.com/hpcc-systems/Visualization/tree/main/packages/markdonw-it-plugins"
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'markdonw' to 'markdown' in the homepage URL path.

Suggested change
"homepage": "https://github.com/hpcc-systems/Visualization/tree/main/packages/markdonw-it-plugins"
"homepage": "https://github.com/hpcc-systems/Visualization/tree/main/packages/markdown-it-plugins"

Copilot uses AI. Check for mistakes.
packages/chart/src/Pie.ts
this._totalValue = this.calcTotalValue();
const innerRadius = this.calcInnerRadius();
const outerRadius = this.calcOuterRadius();
const innerRadius = Math.max(this.calcInnerRadius(), Math.min(outerRadius / 30, 6));
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

The magic numbers 30 and 6 should be defined as named constants to clarify their purpose (e.g., MIN_INNER_RADIUS_RATIO = 30, MIN_INNER_RADIUS_PIXELS = 6).

Copilot uses AI. Check for mistakes.
packages/chart/src/Pie.ts
const paddingValue = this.slicePadding();

// Use a small pad angle for visual separation
const padAngleRadians = paddingValue > 0 ? Math.min(paddingValue, 0.05) : 0;
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

The magic number 0.05 should be defined as a named constant (e.g., MAX_PAD_ANGLE = 0.05) to clarify the maximum allowed pad angle.

Copilot uses AI. Check for mistakes.
packages/chart/src/Column.ts
.domain(context.layerColumns(host).filter(function (_d, idx) { return idx > 0; }))
.rangeRound(isHorizontal ? [0, dataLen] : [dataLen, 0])
.paddingInner(this.xAxisSeriesPaddingInner())
.paddingInner(Math.max(this.xAxisSeriesPaddingInner(), 0.05))
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

The magic number 0.05 should be defined as a named constant (e.g., MIN_SERIES_PADDING = 0.05) to clarify the minimum padding value.

Copilot uses AI. Check for mistakes.
@GordonSmith GordonSmith force-pushed the main branch 3 times, most recently from ac285a3 to 136469b Compare October 29, 2025 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@GordonSmith