Repository navigation
Conversation
Keep click capture inside the panel while retaining the keyboard boundary. Cover veil clicks, URL cleanup and nested dialog dismissal. Refs #4440
Resolve validation popovers through aria-controls and refresh four reviewed Linux ErrorDisplay snapshots. Refs #4440
Reuse the Healthcheck fixture and keep focus restoration checks alongside the browser interaction matrix. Check reload once per opening order and make the external-close test detect focus stealing.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The opt-in behavior preserves existing defaults and is supported by focused unit and cross-browser regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Adds opt-in non-modal Healthcheck drawers to satisfy #4440 while preserving existing modal behavior.
Changes:
- Adds
healthcheck.disableModalwith scoped Escape and focus restoration. - Improves drawer naming, portal click handling, and accessibility coverage.
- Upgrades Gravity UIKit to 7.48.0 and updates affected tests.
| File | Description |
|---|---|
src/components/Drawer/Drawer.tsx |
Implements non-modal drawer behavior and naming. |
src/components/Drawer/Drawer.scss |
Styles the event boundary wrapper. |
src/components/Drawer/__test__/DrawerAccessibility.test.tsx |
Tests focus restoration and adjacent panels. |
src/components/Search/__test__/Search.test.tsx |
Adds required UIKit theme context. |
src/containers/Tenant/Healthcheck/components/HealthcheckDrawer.tsx |
Applies the Healthcheck configuration. |
src/containers/Tenant/Healthcheck/components/__test__/HealthcheckDrawerExtension.test.tsx |
Tests modal configuration propagation. |
src/index.tsx |
Loads the E2E drawer fixture. |
src/types/window.d.ts |
Types the E2E fixture mode. |
src/uiFactory/types.ts |
Exposes the consumer setting. |
tests/fixtures/healthcheckDrawer.tsx |
Provides paired-panel test scenarios. |
tests/suites/sidebar/drawerBehavior.test.ts |
Adds real veil-dismissal regressions. |
tests/suites/sidebar/healthcheckAccessibility.test.ts |
Covers accessibility and focus behavior. |
tests/suites/tenant/queryEditor/models/SettingsDialog.ts |
Resolves error popovers through ARIA links. |
tests/suites/tenant/queryEditor/querySettings.test.ts |
Verifies timeout validation messages. |
tests/utils/clickDrawerVeil.ts |
Adds hit-tested veil clicking. |
package.json |
Upgrades Gravity UIKit. |
package-lock.json |
Locks UIKit 7.48.0. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| async function openPanel(page: Page, name: Panel) { | ||
| await page.getByRole('button', {name: `Open ${name}`, exact: true}).focus(); |
There was a problem hiding this comment.
The new browser checks focus the Open Healthcheck button before opening the drawer. The real compact opener is a clickable Label, so these checks never test focus return from it. Add a non-modal check through the real opener; otherwise a focus-return failure on the page could go unnoticed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/suites/sidebar/healthcheckAccessibility.test.ts
Line: 23-24
Comment:
**Real opener focus goes untested**
The new browser checks focus the `Open Healthcheck` button before opening the drawer. The real compact opener is a clickable `Label`, so these checks never test focus return from it. Add a non-modal check through the real opener; otherwise a focus-return failure on the page could go unnoticed.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
 No flows tested, and faced 1 obstacle. Obstacles faced
To reduce obstacles, configure your TREX environment. |
Opening Healthcheck currently hides adjacent panels and their openers from assistive technology. Add the optional
healthcheck.disableModalsetting so consumers can keep Healthcheck, adjacent panels and the page accessible together. Existing installations retain modal behavior by default.The opt-in mode stays open when focus leaves, scopes Escape to the focused panel and restores the opener without stealing focus. Drawer names come from their headings. Click capture stays inside the panel content, including nested portals, so veil and sidebar clicks retain their existing dismissal behavior.
UIKit is upgraded to 7.48.0; other locked dependencies are unchanged. Query Settings E2E checks follow each error icon's
aria-controlslink, and the four affected Linux ErrorDisplay snapshots are updated for the new Alert rendering.Usage
Configure before rendering the application:
This controls focus modality independently of the veil and outside clicks. Consumers must also configure adjacent panels for compatible non-modal behavior.
Verification
mcr.microsoft.com/playwright:v1.58.0-nobleon linux/amd64, with backendhttp://localhost:8765; all four passed again with snapshot updates disabled.Browser coverage uses real UI components and isolated local data/mocks. Internal AI integration, native screen-reader acceptance and package release remain follow-up work.
Refs #4440
CI Results
Test Status: ❌ FAILED
📊 Full Report
Test Changes Summary ✨15 🗑️17
✨ New Tests (15)
🗑️ Deleted Tests (17)
Bundle Size: 🔽
Current: 65.27 MB | Main: 65.30 MB
Diff: 0.03 MB (-0.05%)
✅ Bundle size decreased.
ℹ️ CI Information
No blocking code issue was established; the real compact opener’s focus-return path still needs browser coverage.
Fix with agent prompt
Summary
Healthcheck drawers gain an opt-in non-modal mode, so the page and nearby panels can stay accessible while the drawer is open. The default remains modal, and the change also upgrades UIKit and updates related UI checks.
healthcheck.disableModaland keeps focus, Escape, and dismissal behavior scoped to each drawer.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Open Healthcheck] --> B{Non-modal setting enabled?} B -- No --> C[Keep modal focus behavior] B -- Yes --> D[Keep page and other panels reachable] D --> E[Focus leaves: keep Healthcheck open] D --> F[Escape inside Healthcheck: close it] F --> G[Return focus when safe]Reviews (2) · Last reviewed commit: "test(healthcheck): trim duplicate drawer..."