Skip to content

Fix focus trapping, className updates, and toast lifetimes (0.6.0) - #7

Merged
cmm-cmm merged 1 commit into
mainfrom
claude/forgedialog-library-mhkpak
Aug 16, 2026
Merged

cmm-cmm merged 1 commit into
mainfrom
claude/forgedialog-library-mhkpak

Conversation

@cmm-cmm

@cmm-cmm cmm-cmm commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Summary

Audit of the whole project followed by fixes for the real defects it surfaced, then a 0.6.0 version bump via Changesets.

The baseline was already green (npm run validate passed, 155 tests), so this focuses on bugs the existing suite did not cover.

Bug fixes

  • Focus trap could never engage. getFocusableElements matched input:not([disabled]), which includes <input type="hidden">. A dialog whose form starts with a hidden field — a CSRF token, for example — focused that input, so document.activeElement stayed on <body> and focus escaped the dialog entirely. Hidden inputs and anything inside a [hidden], [inert], or aria-hidden="true" subtree are now excluded. summary, iframe, audio[controls], video[controls], and contenteditable targets are now recognised. Detection stays attribute-based rather than layout-based so it behaves identically under jsdom.
  • update({ className }) threw. The class string was split without dropping empty tokens, so a padded, empty, or whitespace-only value reached classList and raised SyntaxError: The token provided must not be empty.
  • Toasts could not be made persistent. duration went straight to setTimeout, which coerces 0 and Infinity to 0, so those toasts were removed on the next tick instead of waiting for an explicit dismiss. Both now mean "keep until dismissed".
  • Toast timer leak. Dismissing through an action button removed the element but left the auto-dismiss timer pending.
  • Hardcoded region name. The toast region's accessible name is now localizable through a new optional notifications entry in setLabels().
  • resetForTests() left the body scroll locked, leaking lock state across tests.

Maintenance

  • vitest.config.ts hardcoded __FORGEDIALOG_VERSION__ as '0.5.0' while tsup read it from package.json; it now reads package.json too, so the version cannot drift after a bump.
  • Generated API reports were written with CRLF while the checked-in copies use LF, so api:update produced ~1100-line phantom diffs. Reports are now pinned to LF in both api-extractor.json and scripts/check-subpath-apis.mjs, plus a .gitattributes normalizing the repo.
  • npm audit fix — 5 advisories (1 moderate, 4 high, all transitive dev dependencies) down to 0. Lockfile only; no dependency ranges changed.

Release

changeset version bumps 0.5.0 → 0.6.0 (minor: new optional DialogLabels.notifications, plus the toast duration behavior change). The generated CHANGELOG entry was reformatted to match the flat bullet style used by previous releases.

Test plan

  • npm run validate passes end to end (typecheck, lint, coverage, size budgets, tree-shaking, publint/ATTW, API reports, subpath APIs, consumer builds, CSP, tarball size)
  • npm test — 162 tests pass, up from 155; coverage 91.03% → 91.75% statements
  • Seven regression tests added, each pinned to a defect verified beforehand: hidden-input focus falling through to <body>, hidden/inert/aria-hidden subtrees, the newly recognised focusable types, the classList SyntaxError, persistent toast durations, auto-dismiss plus action-button timer cleanup, and the localized region label
  • npx playwright test --project=chromium — 11 passed, 1 skipped (mobile-only), including the Axe WCAG A/AA check and the focus-trap specs, run against the pre-installed Chromium
  • npm audit — 0 vulnerabilities

Note: npm run test:e2e cannot run unmodified in this environment — the pinned @playwright/test expects browser build 1228 while only 1194 is installed, and downloading browsers is disabled here. The suite was run against the pre-installed Chromium via an executablePath override; CI installs its own browsers and is unaffected.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MtJ3nDatGiv7dHjvZL1uZ4


Generated by Claude Code

Focus detection returned targets that cannot actually receive focus. A
dialog whose form began with a hidden input, such as a CSRF token, focused
that input, which left document.activeElement on <body> so the trap never
engaged. Hidden inputs and anything inside a [hidden], [inert], or
aria-hidden subtree are now excluded, and summary, iframe, media, and
contenteditable targets are recognised.

update({ className }) split the class string without dropping empty tokens,
so a padded or whitespace-only value made classList throw a SyntaxError.

toast() passed its duration straight to setTimeout, which coerces 0 and
Infinity to 0 and dismissed those toasts immediately instead of keeping
them until dismissed. Its auto-dismiss timer also leaked when the toast was
removed through an action button, and the region name was hardcoded English.

Also fixes the dialog stack test reset leaving the body scroll locked, reads
the test-time version from package.json so it cannot drift from the build,
pins generated API reports to LF, and refreshes dev dependencies to clear
all reported advisories.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MtJ3nDatGiv7dHjvZL1uZ4
@coderabbitai

coderabbitai Bot commented Aug 16, 2026 •

Copy link
Copy Markdown

Important

Review available on request

  • 🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 15a5e09d-bfec-4938-ab9b-736ea6c3adf1


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.

@cmm-cmm
cmm-cmm marked this pull request as ready for review August 16, 2026 09:09
@cmm-cmm
cmm-cmm merged commit 7f1fe00 into main Aug 16, 2026
10 of 11 checks passed
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.

2 participants