Skip to content

fix(z-index): drop tabIndex from z-index portal groups to stop pointer focus rings - #7805

Merged
PavelVanecek merged 1 commit into
recharts:mainfrom
devtechedge:fix/zindex-layer-pointer-focus
Sep 13, 2026
Merged

fix(z-index): drop tabIndex from z-index portal groups to stop pointer focus rings#7805
PavelVanecek merged 1 commit into
recharts:mainfrom
devtechedge:fix/zindex-layer-pointer-focus

Conversation

@devtechedge

@devtechedge devtechedge commented Sep 12, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Fixes #7799.

tabindex="-1" on the z-index layer <g> elements makes them pointer-focusable. Clicking anywhere in a chart focuses the nearest layer group, and any app with a global :focus-visible rule then paints a focus ring on it. In WebKit an outline on an SVG element traces its geometry, which looks like a selected bar or ReferenceArea.

This removes the tabIndex={-1} prop from ZIndexSvgPortal. A bare SVG <g> is neither tabbable nor pointer-focusable, so the intent documented in PR #6687 (preventing extraneous focusable surfaces) still holds without the attribute.

Tests: the AllZIndexPortals spec now selects layer groups by their recharts-zIndex-layer_ class instead of by tabindex, and adds an assertion that no rendered layer group carries a tabindex attribute. That assertion fails on main and passes with this change.

Note: the issue cites ZIndexPortal.js; the actual file is src/zIndex/ZIndexPortal.tsx.

Summary by CodeRabbit

  • Bug Fixes
    • Updated SVG layer focus behavior so portal layers are not assigned a tabindex attribute, while remaining non-tabbable and non-pointer-focusable.
  • Tests
    • Updated coverage to verify all portal layers render correctly without a tabindex attribute.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 07944a0f-3739-465d-8a98-9ee4d0cfafb0

📥 Commits

Reviewing files that changed from the base of the PR and between 69f6091 and 5424e6c.

📒 Files selected for processing (2)
  • src/zIndex/ZIndexPortal.tsx
  • test/zIndex/AllZIndexPortals.spec.tsx

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

ZIndexSvgPortal no longer sets tabIndex={-1} on SVG layer groups. Tests now select portal groups by class and verify that they have no tabindex attribute.

Changes

Z-index portal focusability

Layer / File(s) Summary
Remove SVG layer tabindex
src/zIndex/ZIndexPortal.tsx, test/zIndex/AllZIndexPortals.spec.tsx		 The portal renders bare <g> elements without tabIndex={-1}. Tests select layers by class, verify portal rendering, and assert that no tabindex attribute is present.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 5424e

The change removes unwanted portal focusability and updates tests to verify the intended behavior; it is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: removing tabIndex from z-index portal groups to prevent pointer focus rings.
Description check ✅ Passed The description explains the issue, motivation, implementation, linked issue, affected file, and test coverage. It omits the template headings for change types and checklist items, but the required te…
Linked Issues check ✅ Passed The changes satisfy #7799. src/zIndex/ZIndexPortal.tsx removes tabIndex={-1} from the z-index layer <g> element and documents that a bare <g> is not tabbable or pointer-focusable. `test/zIndex…
Out of Scope Changes check ✅ Passed The changes are limited to the implementation and tests for #7799. The test selector changes support verification of the removed attribute. No unrelated behavior or files are changed.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Bundle Report

Changes will increase total bundle size by 10 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
recharts/bundle-cjs 1.6MB 11 bytes (0.0%) ⬆️
recharts/bundle-es6 1.41MB 11 bytes (0.0%) ⬆️
recharts/bundle-umd 622.29kB -12 bytes (-0.0%) ⬇️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-umd

Assets Changed:

Asset Name Size Change Total Size Change (%)
Recharts.js -12 bytes 622.29kB -0.0%
view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
zIndex/ZIndexPortal.js 11 bytes 2.69kB 0.41%
view changes for bundle: recharts/bundle-es6

Assets Changed:

Asset Name Size Change Total Size Change (%)
zIndex/ZIndexPortal.js 11 bytes 1.92kB 0.58%

@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.42%. Comparing base (69f6091) to head (5424e6c).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #7805   +/-   ##
=======================================
  Coverage   88.42%   88.42%           
=======================================
  Files         638      638           
  Lines       15431    15431           
  Branches     4044     4044           
=======================================
  Hits        13645    13645           
  Misses       1591     1591           
  Partials      195      195

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@PavelVanecek
PavelVanecek merged commit 543958b into recharts:main Sep 13, 2026
64 checks passed
devtechedge added a commit to devtechedge/oss-contributions that referenced this pull request Sep 14, 2026
@devtechedge
feat(ledger): automate merge-to-publication cascade
f65fb50 
Record pnpm/pnpm#14863 and recharts/recharts#7805 as merged, add
publications.json plus the Sync merged OSS workflow so README, resume,
LinkedIn, profile fragment, About, and resume artifacts stay in lockstep.
@devtechedge

devtechedge commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for merging, @PavelVanecek.
I learned that tabIndex={-1} on a portal SVG <g> still leaves an empty group reachable, and that WebKit will draw a focus ring around traced geometry because of it.

Removing the attribute and locking the behaviour with a test was a small change with a very specific cause.
Thanks for taking it.

@shin4488 shin4488 mentioned this pull request Oct 30, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PavelVanecek PavelVanecek PavelVanecek approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

tabindex="-1" on the z-index layer groups makes every chart layer mouse-focusable

2 participants

@devtechedge @PavelVanecek