Skip to content

chore: Zoom controls focusing idea - #284

Merged
srungta08 merged 1 commit into
dev-v3-srungta-zoom-keyboardfrom
dev-v3-srungta-zoom-keyboard-focus-proposal
Nov 12, 2026
Merged

srungta08 merged 1 commit into
dev-v3-srungta-zoom-keyboardfrom
dev-v3-srungta-zoom-keyboard-focus-proposal

Conversation

@pan-kot

@pan-kot pan-kot commented Nov 12, 2026

Copy link
Copy Markdown
Member

Description

See: #278 (comment)

How has this been tested?

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pan-kot
pan-kot requested a review from a team as a code owner November 12, 2026 07:58
@pan-kot
pan-kot requested review from amanabiy and srungta08 and removed request for a team and amanabiy November 12, 2026 07:58
@pan-kot
pan-kot requested a balanced review from Copilot November 12, 2026 08:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain for the zoom-control focus refactor.

No open issues 🎉

What changed in this PR

Refactors focus handling for the internal chart zoom controls, moving it from React effects into the zoom state machine and chart helper.

Changes:

  • Adds explicit focus targets for entering, cancelling, and committing zoom selections.
  • Registers button references with the chart API and keeps Reset mounted so it can receive focus before state renders.
  • Updates control spacing, state-machine tests, and test helpers.
File Description
src/​core/​styles.scss Adds spacing between zoom controls.
src/​core/​components/​core-zoom-controls.tsx Registers focus references and keeps Reset mounted in a hidden wrapper.
src/​core/​chart-zoom/​zoom-state-machine.ts Replaces operation notifications with explicit focus effects.
src/​core/​chart-zoom/​__tests__/​zoom-state-machine.test.ts Updates expected effects and adds blur-cancellation coverage.
src/​core/​chart-api/​index.tsx Exposes zoom-button reference setters.
src/​core/​chart-api/​chart-extra-zoom.ts Centralizes focus handling for zoom transitions and reset.
src/​core/​__tests__/​common.tsx Excludes hidden Reset buttons from test lookups.

Review effort: Balanced

@srungta08
srungta08 merged commit 4c02d0d into dev-v3-srungta-zoom-keyboard Nov 12, 2026
4 checks passed
@srungta08
srungta08 deleted the dev-v3-srungta-zoom-keyboard-focus-proposal branch November 12, 2026 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants