fix: recognize visible Dockview chat panels (#3481)
This commit is contained in:
@@ -185,6 +185,7 @@ test.describe("Unread divider", () => {
|
||||
expect(await isScrolledIntoView(scrollContainer, newestRow)).toBe(false);
|
||||
});
|
||||
|
||||
// @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.15
|
||||
test("completed task switch keeps each read cursor and returns to the bottom", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
@@ -214,6 +215,7 @@ test.describe("Unread divider", () => {
|
||||
element.scrollTop = element.scrollHeight;
|
||||
element.dispatchEvent(new Event("scroll"));
|
||||
});
|
||||
await testPage.getByTestId("dockview-tab-changes").click();
|
||||
|
||||
const taskBMarkRead = testPage.waitForResponse(
|
||||
(response) =>
|
||||
@@ -237,6 +239,19 @@ test.describe("Unread divider", () => {
|
||||
|
||||
const activeChat = session.activeChat();
|
||||
const scrollContainer = activeChat.locator(".chat-message-list");
|
||||
await expect
|
||||
.poll(() =>
|
||||
testPage.evaluate(
|
||||
() =>
|
||||
(
|
||||
window as unknown as {
|
||||
__dockviewApi__?: { activePanel?: { id: string } };
|
||||
}
|
||||
).__dockviewApi__?.activePanel?.id ?? null,
|
||||
),
|
||||
)
|
||||
.toBe("changes");
|
||||
await expect(scrollContainer).toBeVisible();
|
||||
await expect(activeChat.getByTestId("unread-divider")).toHaveCount(0);
|
||||
await expect
|
||||
.poll(() =>
|
||||
|
||||
@@ -10,34 +10,53 @@ afterEach(() => {
|
||||
|
||||
type FakePanelApi = {
|
||||
isActive: boolean;
|
||||
isVisible: boolean;
|
||||
onDidActiveChange: (cb: (event: { isActive: boolean }) => void) => { dispose: () => void };
|
||||
onDidVisibilityChange: (cb: (event: { isVisible: boolean }) => void) => {
|
||||
dispose: () => void;
|
||||
};
|
||||
};
|
||||
|
||||
type FakeApiHandle = {
|
||||
fakeApi: FakePanelApi;
|
||||
fireActiveChange: (isActive: boolean) => void;
|
||||
fireVisibilityChange: (isVisible: boolean) => void;
|
||||
};
|
||||
|
||||
function makeFakeApi(initialIsActive: boolean): FakeApiHandle {
|
||||
let listener: ((event: { isActive: boolean }) => void) | null = null;
|
||||
function makeFakeApi(initialIsActive: boolean, initialIsVisible = initialIsActive): FakeApiHandle {
|
||||
let activeListener: ((event: { isActive: boolean }) => void) | null = null;
|
||||
let visibilityListener: ((event: { isVisible: boolean }) => void) | null = null;
|
||||
const fakeApi: FakePanelApi = {
|
||||
isActive: initialIsActive,
|
||||
isVisible: initialIsVisible,
|
||||
onDidActiveChange: (cb) => {
|
||||
listener = cb;
|
||||
return { dispose: () => (listener = null) };
|
||||
activeListener = cb;
|
||||
return { dispose: () => (activeListener = null) };
|
||||
},
|
||||
onDidVisibilityChange: (cb) => {
|
||||
visibilityListener = cb;
|
||||
return { dispose: () => (visibilityListener = null) };
|
||||
},
|
||||
};
|
||||
return {
|
||||
fakeApi,
|
||||
fireActiveChange: (isActive: boolean) => {
|
||||
fakeApi.isActive = isActive;
|
||||
listener?.({ isActive });
|
||||
activeListener?.({ isActive });
|
||||
},
|
||||
fireVisibilityChange: (isVisible: boolean) => {
|
||||
fakeApi.isVisible = isVisible;
|
||||
visibilityListener?.({ isVisible });
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function acquirePanel(panelId: string, initialIsActive: boolean): FakeApiHandle {
|
||||
const handle = makeFakeApi(initialIsActive);
|
||||
function acquirePanel(
|
||||
panelId: string,
|
||||
initialIsActive: boolean,
|
||||
initialIsVisible = initialIsActive,
|
||||
): FakeApiHandle {
|
||||
const handle = makeFakeApi(initialIsActive, initialIsVisible);
|
||||
// Test double: only the subset of DockviewPanelApi this hook actually
|
||||
// reads/calls is implemented. Unchecked cast is intentional here — the
|
||||
// full interface has 30+ unrelated members no test in this file exercises.
|
||||
@@ -57,7 +76,7 @@ describe("usePanelActive", () => {
|
||||
expect(result.current).toBe(false);
|
||||
});
|
||||
|
||||
it("reflects the panel's initial isActive state once registered", () => {
|
||||
it("reflects the panel's initial isVisible state once registered", () => {
|
||||
acquirePanel("panel-active", true);
|
||||
const { result } = renderHook(() => usePanelActive("panel-active"));
|
||||
expect(result.current).toBe(true);
|
||||
@@ -67,15 +86,31 @@ describe("usePanelActive", () => {
|
||||
expect(result2.current).toBe(false);
|
||||
});
|
||||
|
||||
it("updates when the panel's active tab changes via onDidActiveChange", () => {
|
||||
it("treats a selected panel as visible when another Dockview group owns focus", () => {
|
||||
const handle = acquirePanel("panel-visible-unfocused", false, true);
|
||||
const { result } = renderHook(() => usePanelActive("panel-visible-unfocused"));
|
||||
|
||||
expect(result.current).toBe(true);
|
||||
|
||||
// The hook subscribes to onDidVisibilityChange, not onDidActiveChange.
|
||||
// Firing active-change events is a no-op; the result must not move.
|
||||
act(() => handle.fireActiveChange(true));
|
||||
act(() => handle.fireActiveChange(false));
|
||||
expect(result.current).toBe(true);
|
||||
|
||||
act(() => handle.fireVisibilityChange(false));
|
||||
expect(result.current).toBe(false);
|
||||
});
|
||||
|
||||
it("updates when the panel's visible tab changes via onDidVisibilityChange", () => {
|
||||
const handle = acquirePanel("panel-toggle", false);
|
||||
const { result } = renderHook(() => usePanelActive("panel-toggle"));
|
||||
expect(result.current).toBe(false);
|
||||
|
||||
act(() => handle.fireActiveChange(true));
|
||||
act(() => handle.fireVisibilityChange(true));
|
||||
expect(result.current).toBe(true);
|
||||
|
||||
act(() => handle.fireActiveChange(false));
|
||||
act(() => handle.fireVisibilityChange(false));
|
||||
expect(result.current).toBe(false);
|
||||
});
|
||||
|
||||
@@ -103,13 +138,13 @@ describe("usePanelActive", () => {
|
||||
// without the old one ever being released.
|
||||
const fresh = acquirePanel(PANEL_ID, false);
|
||||
|
||||
act(() => fresh.fireActiveChange(true));
|
||||
act(() => fresh.fireVisibilityChange(true));
|
||||
expect(result.current).toBe(true);
|
||||
|
||||
// The stale api's listener must have been torn down — firing it no
|
||||
// longer moves the hook (it would incorrectly flip back to false
|
||||
// without the resubscribe fix).
|
||||
act(() => stale.fireActiveChange(false));
|
||||
act(() => stale.fireVisibilityChange(false));
|
||||
expect(result.current).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,7 +5,7 @@ import type { DockviewPanelApi } from "dockview-react";
|
||||
import { panelPortalManager } from "@/lib/layout/panel-portal-manager";
|
||||
|
||||
/**
|
||||
* Tracks whether the dockview panel identified by panelId is the active tab
|
||||
* Tracks whether the dockview panel identified by panelId is the visible tab
|
||||
* in its dockview group. Portal-hosted panel content (see
|
||||
* `lib/layout/panel-portal-manager.ts`) stays mounted while its tab is
|
||||
* inactive — e.g. the Chat tab keeps running behind an active Files/Changes
|
||||
@@ -19,8 +19,10 @@ import { panelPortalManager } from "@/lib/layout/panel-portal-manager";
|
||||
* renders, so the entry is virtually always present by the time this runs.
|
||||
* Subscribes to both the manager's own add/remove notifications (a panel
|
||||
* acquired after this hook's first mount, or released while it stays
|
||||
* mounted) and the panel's own `onDidActiveChange` (tab switches within an
|
||||
* existing group).
|
||||
* mounted) and the panel's own `onDidVisibilityChange` (tab switches within
|
||||
* an existing group). Dockview's `isActive` also requires the panel's group
|
||||
* to own global focus, so it is not a valid proxy for whether the panel is
|
||||
* rendered on screen beside another focused group.
|
||||
*
|
||||
* Only for dockview-hosted panels — pass the panel's real id. Defaults to
|
||||
* `false` until a portal entry/api is actually registered for it, rather
|
||||
@@ -34,7 +36,7 @@ import { panelPortalManager } from "@/lib/layout/panel-portal-manager";
|
||||
export function usePanelActive(panelId: string): boolean {
|
||||
const subscribe = useCallback(
|
||||
(onStoreChange: () => void) => {
|
||||
// Tracks which api our onDidActiveChange listener is currently bound
|
||||
// Tracks which api our onDidVisibilityChange listener is currently bound
|
||||
// to, so a manager notification (panel acquired/released/remounted
|
||||
// anywhere) can detect when *this* panelId's api was replaced —
|
||||
// Dockview does this during a layout restore/switch — and rebind
|
||||
@@ -47,7 +49,7 @@ export function usePanelActive(panelId: string): boolean {
|
||||
if (api === boundApi) return;
|
||||
disposable?.dispose();
|
||||
boundApi = api;
|
||||
disposable = api?.onDidActiveChange(onStoreChange) ?? null;
|
||||
disposable = api?.onDidVisibilityChange(onStoreChange) ?? null;
|
||||
};
|
||||
|
||||
rebindIfApiChanged();
|
||||
@@ -63,7 +65,7 @@ export function usePanelActive(panelId: string): boolean {
|
||||
[panelId],
|
||||
);
|
||||
const getSnapshot = useCallback(
|
||||
() => panelPortalManager.get(panelId)?.api?.isActive ?? false,
|
||||
() => panelPortalManager.get(panelId)?.api?.isVisible ?? false,
|
||||
[panelId],
|
||||
);
|
||||
return useSyncExternalStore(subscribe, getSnapshot, () => false);
|
||||
|
||||
@@ -41,6 +41,14 @@ A focused throwaway hook test reproduced the race: session A's delayed response
|
||||
produced no `updateSessionReadCursor("session-1", "m2")` call after session B
|
||||
dispatched. The temporary test was removed after recording the red evidence.
|
||||
|
||||
The follow-up trace exposed a separate visibility defect. Dockview restored the
|
||||
incoming Chat tab as selected in the center group while the side-by-side
|
||||
Changes group retained global focus. `usePanelActive` read Dockview's
|
||||
`isActive`, which combines group-local tab visibility with global group focus,
|
||||
so the visibly rendered transcript reported `isVisible=false` and skipped
|
||||
initial placement. Completed agents produced no later message or work-state
|
||||
write to mask the skipped placement.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
@@ -119,6 +127,7 @@ changes.
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Preserve read cursor across task switches](task-01-preserve-read-cursor-across-task-switches.md)
|
||||
- [x] [Task 02: Recognize visible Dockview chat panels](task-02-recognize-visible-dockview-chat-panels.md)
|
||||
|
||||
## Verification results
|
||||
|
||||
@@ -132,6 +141,15 @@ changes.
|
||||
- Mobile Chrome completed-task switch E2E passed (1 test).
|
||||
- Specification linter tests passed (30 tests); all specification files passed.
|
||||
- `git diff --check` passed.
|
||||
- Follow-up TDD regression: the visibility hook returned false when Dockview
|
||||
reported `isActive=false` and `isVisible=true`; it passed after switching to
|
||||
the group-local visibility contract.
|
||||
- Follow-up focused Vitest passed: 1 file, 6 tests.
|
||||
- Follow-up TypeScript typecheck and scoped ESLint passed.
|
||||
- Follow-up Chromium task-switch E2E passed with Changes retaining global
|
||||
Dockview focus while the visible Chat transcript returned to the bottom.
|
||||
- Follow-up Mobile Chrome completed-task switch E2E passed.
|
||||
- Follow-up specification linter tests and full specification lint passed.
|
||||
|
||||
## Risks
|
||||
|
||||
|
||||
+97
@@ -0,0 +1,97 @@
|
||||
---
|
||||
id: "02-recognize-visible-dockview-chat-panels"
|
||||
title: "Recognize visible Dockview chat panels"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on:
|
||||
- "01-preserve-read-cursor-across-task-switches"
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-UI-TRANSCRIPT-AUTO-SCROLL-001
|
||||
acceptance_criteria:
|
||||
- AC-UI-TRANSCRIPT-AUTO-SCROLL-001.11
|
||||
- AC-UI-TRANSCRIPT-AUTO-SCROLL-001.14
|
||||
- AC-UI-TRANSCRIPT-AUTO-SCROLL-001.15
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/transcript-auto-scroll.md
|
||||
---
|
||||
|
||||
# Task 02: Recognize Visible Dockview Chat Panels
|
||||
|
||||
## Summary
|
||||
|
||||
Treat the selected Chat tab as visible independently of which side-by-side
|
||||
Dockview group owns global focus. Prove that a completed task restores its
|
||||
transcript placement without relying on a later agent update.
|
||||
|
||||
## In scope
|
||||
|
||||
- Subscribe to Dockview panel visibility rather than global active state.
|
||||
- Cover the focused-sibling-group contract in the visibility hook.
|
||||
- Extend the desktop completed-task switch regression with Changes-group focus.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Dockview layout persistence or active-group restoration.
|
||||
- Transcript auto-scroll preference semantics or stored offsets.
|
||||
- Mobile composition, which does not mount Dockview.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A selected Chat tab reports visible when another Dockview group owns focus.
|
||||
- Hiding Chat behind another tab in its own group reports not visible.
|
||||
- Returning to an idle completed task places the transcript at the bottom while
|
||||
Changes remains the globally active panel.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps && pnpm --filter @kandev/web test -- --run hooks/use-panel-active.test.ts)
|
||||
(cd apps/web && pnpm exec eslint hooks/use-panel-active.ts hooks/use-panel-active.test.ts e2e/tests/chat/unread-divider.spec.ts)
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
(cd apps/web && pnpm e2e:run --host --project chromium tests/chat/unread-divider.spec.ts -- --grep "completed task switch" --retries=0)
|
||||
(cd apps/web && pnpm e2e:run --host --no-build --project mobile-chrome tests/chat/mobile-unread-divider.spec.ts -- --grep "completed task switch" --retries=0)
|
||||
python3 scripts/lint-spec-files.test.py
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/hooks/use-panel-active.ts`
|
||||
- `apps/web/hooks/use-panel-active.test.ts`
|
||||
- `apps/web/e2e/tests/chat/unread-divider.spec.ts`
|
||||
- `docs/specs/ui/requirements/transcript-auto-scroll.md`
|
||||
- `docs/specs/ui/system-design/transcript-auto-scroll.md`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 01 established the environment-switch placement lifecycle and its
|
||||
completed-task regression.
|
||||
|
||||
## Risks
|
||||
|
||||
- Using global Dockview focus recreates the defect when another visible group
|
||||
remains active after layout restoration.
|
||||
- Treating every mounted portal as visible could mark hidden session tabs read.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- `REQ-UI-TRANSCRIPT-AUTO-SCROLL-001`, especially acceptance criteria `.11`,
|
||||
`.14`, and `.15`.
|
||||
- UI transcript auto-scroll system design.
|
||||
- Dockview panel visibility and active-state contracts.
|
||||
|
||||
## Results
|
||||
|
||||
- Replaced Dockview's global active-state subscription with the group-local
|
||||
visibility property and event.
|
||||
- Added unit coverage for a visible Chat panel whose sibling group owns focus.
|
||||
- Extended the desktop completed-task switch regression to retain Changes as
|
||||
the active panel and prove the visible transcript returns to the bottom.
|
||||
- Focused Vitest, scoped ESLint, TypeScript typecheck, desktop Chromium E2E,
|
||||
Mobile Chrome parity E2E, specification lint, and diff checks passed.
|
||||
@@ -2,7 +2,7 @@
|
||||
status: active
|
||||
system: ui
|
||||
created: 2026-07-30
|
||||
updated: 2026-09-04
|
||||
updated: 2026-09-07
|
||||
owners:
|
||||
- cfl
|
||||
---
|
||||
@@ -72,6 +72,11 @@ hand-off it is designed to protect.
|
||||
newest cached message and a disabled transcript shows its saved reader
|
||||
position as soon as the panel is measurable, without first exposing the
|
||||
browser's default top position.
|
||||
- **AC-UI-TRANSCRIPT-AUTO-SCROLL-001.15:** **GIVEN** a desktop transcript is the
|
||||
selected tab in its Dockview group while another visible group owns global
|
||||
focus, **WHEN** a task switch restores that layout, **THEN** the transcript
|
||||
is treated as visible and completes its enabled or disabled initial
|
||||
placement.
|
||||
|
||||
## Migrated source detail
|
||||
|
||||
@@ -127,6 +132,9 @@ the asynchronous hand-off it is designed to protect.
|
||||
**WHEN** the user switches between them and returns, **THEN** the incoming
|
||||
enabled transcript shows the newest cached message immediately and remains
|
||||
at the newest message after its history refresh.
|
||||
- **GIVEN** the Changes panel owns Dockview focus while Chat remains the
|
||||
selected center tab, **WHEN** the user returns to that task, **THEN** Chat
|
||||
completes initial transcript placement without requiring an agent update.
|
||||
- **GIVEN** the incoming transcript has auto-scroll disabled, **WHEN** that
|
||||
environment-switch placement runs, **THEN** it restores the incoming
|
||||
session's saved position rather than the outgoing transcript's position.
|
||||
|
||||
@@ -4,7 +4,7 @@ system: ui
|
||||
requirements:
|
||||
- REQ-UI-TRANSCRIPT-AUTO-SCROLL-001
|
||||
created: 2026-08-27
|
||||
updated: 2026-09-04
|
||||
updated: 2026-09-07
|
||||
owners:
|
||||
- kandev
|
||||
---
|
||||
@@ -50,8 +50,15 @@ if that path reads it. The scroll setter records the WebKit-safe offset.
|
||||
Desktop Dockview renders panel content through persistent portals. An inactive
|
||||
session transcript can therefore be mounted and receive messages while its
|
||||
scroll container has zero or detached geometry. `usePanelActive` supplies the
|
||||
authoritative active-tab signal to `TaskChatPanel`, which passes it to the
|
||||
native transcript scroll coordinator as `isVisible`.
|
||||
authoritative group-local visibility signal to `TaskChatPanel`, which passes it
|
||||
to the native transcript scroll coordinator as `isVisible`.
|
||||
|
||||
Dockview's panel `isVisible` property means the panel is the selected tab in
|
||||
its own group. Its `isActive` property additionally requires that group to own
|
||||
global Dockview focus. The transcript uses `isVisible` and
|
||||
`onDidVisibilityChange`; clicking a side-by-side Files or Changes group cannot
|
||||
make a still-rendered Chat panel ineligible for initial placement or read
|
||||
tracking.
|
||||
|
||||
Geometry-based initialization and divider placement do not consume their
|
||||
one-time completion state while `isVisible` is false. When a transcript first
|
||||
@@ -194,10 +201,11 @@ pinning, and disabled position restoration.
|
||||
## Failure modes
|
||||
|
||||
- If the container is not mounted, the helper performs no work.
|
||||
- If a transcript is mounted but inactive, one-time placement remains pending
|
||||
until the panel becomes active and measurable.
|
||||
- If a panel becomes inactive again before the post-restore frames complete,
|
||||
the scheduled placement is cancelled.
|
||||
- If a transcript is mounted but hidden behind another tab in its group,
|
||||
one-time placement remains pending until the panel becomes visible and
|
||||
measurable.
|
||||
- If a panel becomes hidden again before the post-restore frames complete, the
|
||||
scheduled placement is cancelled.
|
||||
- Chromium, Gecko, and WebKit clamp the signed 32-bit maximum to the native
|
||||
maximum scroll position.
|
||||
- An offset outside WebKit's safe range can resolve to zero and move the
|
||||
|
||||
Reference in New Issue
Block a user