fix(ui,app): #11174 review follow-ups — no-dock launcher spec + notification-panel Escape deferral - #11211
Conversation
…cape deferral for the desktop notification panel Two required fixes from the #11174 review, landed as a follow-up since the PR was squash-merged before they could be pushed to its branch: 1. packages/app/test/ui-smoke/launcher-interaction.spec.ts still asserted the removed featured-views dock (docked chat/settings, absent from page 0). Updated to the new contract: no launcher-dock element; Chat/Settings are ordinary page tiles leading the curated Apps page (launcher-curation.ts APPS_PAGE_ORDER); chat launches from its page tile. Evidence dir renamed 9144-default-launcher-dock -> 9144-launcher-page-tiles; swipe helper made bidirectional so the test returns to page 0 before launching. 2. ContinuousChatOverlay Escape-deferral selector matched only [data-testid="notification-sheet"], not the new desktop anchored [data-testid="notification-panel"] (role=dialog, no data-state=open), so Escape with the panel open over an open chat sheet closed both. Added the panel testid + a unit test mirroring the transcript-viewer deferral test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
❌ PR title does not match the required pattern. Please use one of these formats:
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
PR #11174 was reviewed MERGE-WITH-FIX with exactly two required fixes, but it was squash-merged (188b73e) before they landed. This follow-up applies both:
1.
launcher-interaction.spec.tsupdated to the no-dock contractThe keyless-CI-denylisted spec still asserted the removed featured-views dock (
launcher-dockvisible, chat/settings docked and absent from page 0,dockTiles.slice(0,2) == [chat, settings]). Rewritten against the launcher code as merged:launcher-dockhas count 0 (contract enforced inLauncher.test.tsx/LauncherSurface.test.tsxandrun-home-screen-e2e.mjstoo);launcher-curation.tsAPPS_PAGE_ORDER = ["chat", "settings", …], curated groups passed viapageGroups);launcher-pager-edge-*fallback) so the test returns to page 0 first — inactive pages areinert;9144-default-launcher-dock→9144-launcher-page-tiles.2. Escape deferral for the desktop anchored notification panel
ContinuousChatOverlay.tsxEscape-collapse guard matched[data-testid="notification-sheet"]but not the new[data-testid="notification-panel"](role="dialog", nodata-state="open"— not Radix), so Escape with the panel open over an open chat sheet closed both. Added the panel testid to the selector and a unit test mirroring the existing transcript-viewer deferral test (renders overlay, mounts anotification-paneldialog, asserts Escape leaves the sheet open, then collapses once the panel is gone).Verification
bun run --cwd packages/ui test -- ContinuousChatOverlay.test.tsx— 124 passed (incl. new test)bun run --cwd packages/ui test -- Launcher.test.tsx LauncherSurface.test.tsx Launcher.gestures.test.tsx NotificationCenter.test.tsx HomeLauncherSurface.composed.test.tsx— 56 passedpackages/ui+packages/apptypecheck: only 2 pre-existing errors unrelated to this change (@stwd/sdk0.8.1/0.11.0 duplication in cloud-ui auth, missingiwertypes in spatial__e2e__) — present on develop tip.Launcher.tsx,LauncherSurface.tsx,launcher-curation.ts,useHorizontalPager.ts, andPagerEdgeButtons.tsxas merged.🤖 Generated with Claude Code