Repository navigation
test(BottomSheet): find a non-modal sheet as an open manual popover - #8647
Conversation
Since open-order stacking landed, a non-modal sheet (`hasScrim={false}`) is
presented in the top layer as a manual popover (component:BottomSheet FR11),
which carries no `open` attribute. The nested-Popover Escape case looked for
an open sheet as `dialog[open]`, so its non-modal run found none and failed in
Chromium and WebKit before reaching the Escape presses it checks.
The locator now matches an open dialog or an open popover. Escape behaves as
the case asserts: the first press closes the nested Popover and keeps the
sheet, the second closes the sheet. The modal and required cases are
unchanged.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
There was a problem hiding this comment.
Recommended for approval. The new locator matches how component:BottomSheet FR11 hosts a non-modal sheet: as a manual popover, which carries no open attribute. The non-modal Escape case therefore passes again without any product change, and its final assertion now really observes the sheet closing. Nothing more is needed from you; because this spec is release-only, the release check is where the fix will show.
[Automated review]
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: No accessibility violations detected. Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 35 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
|
Posted by Cindy's agent on her behalf: closed and reopened only to restart this PR's CI on the current workflow after tonight's queue-wide cancel; nothing in the PR changed. |
Why
One of the 27 tests in BottomSheet's keyboard browser suite, "non-modal: Escape closes the nested Popover before the sheet", has failed on main in Chromium and WebKit since sheets started stacking by open order (#7648), so the suite is red on main and on every pull request that runs it.
What led to it
Found while running BottomSheet's browser tests for the keyboard work (part of bringing Astryx's touch behavior to parity with iOS's UIKit, text entry first). The test finds an open sheet with
dialog[open]. Since #7648, a non-modal sheet opens as a popover, ascomponent:BottomSheetFR11 specifies, and a popover doesn't carry theopenattribute. So the test counted zero open sheets, even though the sheet was open and Escape still worked.Options and tradeoffs
openattribute back for non-modal sheets: undoes part of feat(core): BottomSheet stacks by open order, replaces on a same-update switch, and BottomSheetSwitcher is deprecated #7648's intended change.Why this choice
The last one keeps what the test is for (one Escape closes the nested Popover, the next closes the sheet) and only fixes how it finds the sheet. No product code changes.
How to check it
Evidence
dialog[open], dialog:popover-open, with a comment citing FR11.