Skip to content

fix(overlay-modal): Overlay role, Modal RTL swipe, Portal shadow root - #686

Open
blvdmitry wants to merge 5 commits into
fix/selectfrom
fix/overlay-modal
Open

blvdmitry wants to merge 5 commits into
fix/selectfrom
fix/overlay-modal

Conversation

@blvdmitry

Copy link
Copy Markdown
Contributor

Summary

PR 6 of the Phase −1 bug-fix stack.

  • Overlay root role (Overlay/Overlay.tsx): role="button" became role="presentation", and the prefer-tag-over-role disable comment was removed because it's no longer needed. Escape already handles keyboard dismissal. tabIndex={-1} is unchanged (see notes).
  • Modal RTL swipe-to-close (Modal/useModalDrag.ts, Modal/Modal.module.css):
    • closingSign is now based on the side the drawer is attached to: start in LTR and end in RTL close toward negative x.
    • The CSS clamp is mirrored for [dir="rtl"]. Before, it only allowed negative drag for start and positive drag for end, so fixing the JS alone wouldn't render the drag.
    • The browser-back edge guard now applies to the drawer attached to the left side, instead of always position === "start". LTR behaviour is unchanged.
  • Portal in a shadow root (_private/Portal/Portal.tsx): the bug reproduces. The detector <div> is removed after mount, so any later render read rootRef.current === null and moved the portal content to document.body. The detected shadow root is now stored in a ref.
  • Story type fix: a merge from fix/select brings in a fix for a type error in a fix(select): listbox semantics, multiple selection, trigger label, first option value #685 test story (renderValue is required with multiple).

Tests (each of these fails on the base branch and passes here):

  • Overlay.stories.tsx: the className, attributes story now asserts role="presentation".
  • Portal.stories.tsx: new test: shadow root, which re-renders and checks the content stays in the shadow root.
  • Modal.stories.tsx: new test: rtl swipe direction, which uses native Touch events. The existing onClose, onAfterOpen, onAfterClose story found the backdrop via its old button role and now queries presentation.

Related Issue

Phase −1 of the headless-extraction plan.

Screenshots / Recordings

The RTL start/end drawers now follow the finger when swiped to close. No other visual changes.

Notes for Reviewers

  • This PR is stacked on fix(select): listbox semantics, multiple selection, trigger label, first option value #685 (fix/select).
  • Overlay tabIndex={-1}: I kept it to avoid changing focus behaviour when clicking the backdrop. axe has a best-practice rule (presentation-role-conflict) for focusable elements with role="presentation". Dropping tabIndex might be worth a separate look.
  • Flaky test: the Modal > onClose, onAfterOpen, onAfterClose failure in the base-line run on main was a one-off flake. It passes consistently here after the query update.
  • Results: pnpm lint passes and tsc passes. pnpm test:browser has 622 passing and 1 failing, Image > onLoad, which also fails on main in this sandbox because it has no network access.

Generated by Claude Code

This branch has not been deployed

No deployments
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