Skip to content

Commit dfdc234

Browse files
marthakellymarthakelly
authored
fix: respect defaultOpen={false} in CopilotSidebar and CopilotPopup (CopilotKit#3513)
## Summary `<CopilotSidebar defaultOpen={false} />` was opening anyway because the prop was not being forwarded through the provider chain. This PR fixes that - and also addresses the architectural trade-off flagged in review. ### Who is affected **User A** just uses `<CopilotSidebar />` with no outer provider Before the fix, their sidebar opened by default. After the fix, it still opens by default. Nothing changed for them. **User B** uses `<CopilotSidebar defaultOpen={false} />` Before: broken (opened anyway). After: works. **User C** wraps their app in a bare `CopilotChatConfigurationProvider` and reads `isModalOpen` from it. Before our fix `isModalOpen` was stuck at true forever. After: it reflects the sidebar's state. ### What changed - The `defaultOpen` prop is forwarded correctly through the provider chain to both `CopilotSidebar` and `CopilotPopup`. - Outer `useCopilotChatConfiguration()` hooks now reflect sidebar state. Fixed via bidirectional state sync between nested providers. ### Files changed - `packages/v2/react/src/components/chat/CopilotChat.tsx` - accept and forward `isModalDefaultOpen` - `packages/v2/react/src/components/chat/CopilotSidebar.tsx` - pass `isModalDefaultOpen={defaultOpen}` - `packages/v2/react/src/components/chat/CopilotPopup.tsx` - same - `packages/v2/react/src/providers/CopilotChatConfigurationProvider.tsx` - fix resolution logic + bidirectional sync - `packages/v2/react/src/providers/__tests__/CopilotChatConfigurationProvider.test.tsx` - add tests ### Test plan - [x] 843 tests pass (including 4 new bidirectional sync tests) - [x] New tests cover all three reproduction scenarios from [tkt-modal-default-open](https://github.com/CopilotKit/deep-agent-cpk-experiments/tree/main/app/client/src/tickets/tkt-modal-default-open) **Linear:** [CPK-7152](https://linear.app/copilotkit/issue/CPK-7152/fix-defaultopenfalse-prop-ignored-in-copilotsidebar) **GitHub Issue:** CopilotKit#3475 **Related PRs:** CopilotKit#3477, CopilotKit#3496 (stalled community PRs addressing the same issue) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
2 parents 72d637b + ea9b0bb commit dfdc234

6 files changed

Lines changed: 233 additions & 4 deletions

File tree

‎.changeset/fix-default-open-prop.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@copilotkitnext/react": patch
3+
---
4+
5+
fix: respect defaultOpen={false} in CopilotSidebar and CopilotPopup

‎packages/v2/react/src/components/chat/CopilotChat.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export type CopilotChatProps = Omit<
3535
threadId?: string;
3636
labels?: Partial<CopilotChatLabels>;
3737
chatView?: SlotValue<typeof CopilotChatView>;
38+
isModalDefaultOpen?: boolean;
3839
/**
3940
* Error handler scoped to this chat's agent. Fires in addition to the
4041
* provider-level onError (does not suppress it). Receives only errors
@@ -51,6 +52,7 @@ export function CopilotChat({
5152
threadId,
5253
labels,
5354
chatView,
55+
isModalDefaultOpen,
5456
onError,
5557
...props
5658
}: CopilotChatProps) {
@@ -382,6 +384,7 @@ export function CopilotChat({
382384
agentId={resolvedAgentId}
383385
threadId={resolvedThreadId}
384386
labels={labels}
387+
isModalDefaultOpen={isModalDefaultOpen}
385388
>
386389
{transcriptionError && (
387390
<div

‎packages/v2/react/src/components/chat/CopilotPopup.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export function CopilotPopup({
5454
<CopilotChat
5555
welcomeScreen={CopilotPopupView.WelcomeScreen}
5656
{...chatProps}
57+
isModalDefaultOpen={defaultOpen}
5758
chatView={PopupViewOverride}
5859
/>
5960
);

‎packages/v2/react/src/components/chat/CopilotSidebar.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export function CopilotSidebar({
4949
<CopilotChat
5050
welcomeScreen={CopilotSidebarView.WelcomeScreen}
5151
{...chatProps}
52+
isModalDefaultOpen={defaultOpen}
5253
chatView={SidebarViewOverride}
5354
/>
5455
);

‎packages/v2/react/src/providers/CopilotChatConfigurationProvider.tsx‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import React, {
22
createContext,
3+
useCallback,
34
useContext,
45
ReactNode,
6+
useEffect,
57
useMemo,
8+
useRef,
69
useState,
710
} from "react";
811
import { DEFAULT_AGENT_ID, randomUUID } from "@copilotkitnext/shared";
@@ -88,9 +91,44 @@ export const CopilotChatConfigurationProvider: React.FC<
8891
const [internalModalOpen, setInternalModalOpen] =
8992
useState<boolean>(resolvedDefaultOpen);
9093

91-
const resolvedIsModalOpen = parentConfig?.isModalOpen ?? internalModalOpen;
92-
const resolvedSetModalOpen =
93-
parentConfig?.setModalOpen ?? setInternalModalOpen;
94+
const hasExplicitDefault = isModalDefaultOpen !== undefined;
95+
96+
// When this provider owns its modal state, wrap the setter so that changes
97+
// propagate upward to any ancestor provider. This allows an outer
98+
// CopilotChatConfigurationProvider (e.g. a user's layout-level provider) to
99+
// observe open/close events that originate deep in the tree — fixing the
100+
// "outer hook always returns true" regression (CPK-7152 Behavior B).
101+
const setAndSync = useCallback(
102+
(open: boolean) => {
103+
setInternalModalOpen(open);
104+
parentConfig?.setModalOpen(open);
105+
},
106+
// eslint-disable-next-line react-hooks/exhaustive-deps
107+
[parentConfig?.setModalOpen],
108+
);
109+
110+
// Sync parent → child: when an ancestor's modal state is changed externally
111+
// (e.g. the user calls setModalOpen from an outer hook), reflect that change
112+
// in our own state so the sidebar/popup responds accordingly.
113+
// Skip the initial mount so that our own isModalDefaultOpen is respected and
114+
// not immediately overwritten by the parent's current value.
115+
const isMounted = useRef(false);
116+
useEffect(() => {
117+
if (!hasExplicitDefault) return;
118+
if (!isMounted.current) {
119+
isMounted.current = true;
120+
return;
121+
}
122+
if (parentConfig?.isModalOpen === undefined) return;
123+
setInternalModalOpen(parentConfig.isModalOpen);
124+
}, [parentConfig?.isModalOpen, hasExplicitDefault]);
125+
126+
const resolvedIsModalOpen = hasExplicitDefault
127+
? internalModalOpen
128+
: (parentConfig?.isModalOpen ?? internalModalOpen);
129+
const resolvedSetModalOpen = hasExplicitDefault
130+
? setAndSync
131+
: (parentConfig?.setModalOpen ?? setInternalModalOpen);
94132

95133
const configurationValue: CopilotChatConfigurationValue = useMemo(
96134
() => ({

‎packages/v2/react/src/providers/__tests__/CopilotChatConfigurationProvider.test.tsx‎

Lines changed: 182 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from "react";
2-
import { render, screen } from "@testing-library/react";
2+
import { render, screen, fireEvent, act } from "@testing-library/react";
33
import { describe, it, expect } from "vitest";
44
import {
55
CopilotChatConfigurationProvider,
@@ -330,6 +330,187 @@ describe("CopilotChatConfigurationProvider", () => {
330330
expect(screen.getByTestId("isModalOpen").textContent).toBe("closed");
331331
expect(screen.getByTestId("hasSetModalOpen").textContent).toBe("yes");
332332
});
333+
334+
it("should allow nested provider to override parent modal state with explicit isModalDefaultOpen", () => {
335+
function ModalStateDisplay() {
336+
const config = useCopilotChatConfiguration();
337+
return (
338+
<div>
339+
<div data-testid="isModalOpen">
340+
{config?.isModalOpen ? "open" : "closed"}
341+
</div>
342+
</div>
343+
);
344+
}
345+
346+
render(
347+
<CopilotChatConfigurationProvider
348+
threadId="outer-thread"
349+
isModalDefaultOpen={true}
350+
>
351+
<CopilotChatConfigurationProvider
352+
threadId="inner-thread"
353+
isModalDefaultOpen={false}
354+
>
355+
<ModalStateDisplay />
356+
</CopilotChatConfigurationProvider>
357+
</CopilotChatConfigurationProvider>,
358+
);
359+
360+
expect(screen.getByTestId("isModalOpen").textContent).toBe("closed");
361+
});
362+
});
363+
364+
/**
365+
* CPK-7152: Bidirectional sync between nested providers.
366+
*
367+
* The fix must satisfy both:
368+
* Behavior A — child provider respects its own isModalDefaultOpen even
369+
* when a parent provider exists (covered by the existing
370+
* "allow nested provider to override" test above).
371+
* Behavior B — state changes in the inner provider propagate outward so
372+
* that hooks reading from an outer provider stay in sync.
373+
*
374+
* Scenarios mirror the reproduction cases in:
375+
* https://github.com/CopilotKit/deep-agent-cpk-experiments/tree/main/app/client/src/tickets/tkt-modal-default-open
376+
*/
377+
describe("Bidirectional sync (CPK-7152)", () => {
378+
// Reusable probe/control component that reads the closest provider.
379+
function ModalControls({ id }: { id: string }) {
380+
const config = useCopilotChatConfiguration();
381+
return (
382+
<>
383+
<div data-testid={`${id}-state`}>{String(config?.isModalOpen)}</div>
384+
<button
385+
data-testid={`${id}-open`}
386+
onClick={() => config?.setModalOpen(true)}
387+
>
388+
open
389+
</button>
390+
<button
391+
data-testid={`${id}-close`}
392+
onClick={() => config?.setModalOpen(false)}
393+
>
394+
close
395+
</button>
396+
</>
397+
);
398+
}
399+
400+
it("scenario-sidebar-outer-hook: inner setModalOpen propagates to outer hook (Behavior B)", () => {
401+
// Abe.Hu's layout: outer bare provider, inner provider owns explicit state.
402+
// Toggling via the inner provider should update the outer hook.
403+
render(
404+
<CopilotChatConfigurationProvider threadId="outer">
405+
{/* OuterProbe sits outside the inner provider — reads outer context */}
406+
<ModalControls id="outer" />
407+
<CopilotChatConfigurationProvider
408+
threadId="inner"
409+
isModalDefaultOpen={true}
410+
>
411+
<ModalControls id="inner" />
412+
</CopilotChatConfigurationProvider>
413+
</CopilotChatConfigurationProvider>,
414+
);
415+
416+
expect(screen.getByTestId("outer-state").textContent).toBe("true");
417+
expect(screen.getByTestId("inner-state").textContent).toBe("true");
418+
419+
act(() => {
420+
fireEvent.click(screen.getByTestId("inner-close"));
421+
});
422+
423+
// Inner closed — outer hook must reflect the change.
424+
expect(screen.getByTestId("inner-state").textContent).toBe("false");
425+
expect(screen.getByTestId("outer-state").textContent).toBe("false");
426+
});
427+
428+
it("scenario-sidebar-outer-hook: outer setModalOpen propagates to inner (parent→child sync)", () => {
429+
// If the user calls setModalOpen from the outer hook, the inner
430+
// provider (and therefore the sidebar) must respond.
431+
render(
432+
<CopilotChatConfigurationProvider
433+
threadId="outer"
434+
isModalDefaultOpen={false}
435+
>
436+
<ModalControls id="outer" />
437+
<CopilotChatConfigurationProvider
438+
threadId="inner"
439+
isModalDefaultOpen={false}
440+
>
441+
<ModalControls id="inner" />
442+
</CopilotChatConfigurationProvider>
443+
</CopilotChatConfigurationProvider>,
444+
);
445+
446+
expect(screen.getByTestId("outer-state").textContent).toBe("false");
447+
expect(screen.getByTestId("inner-state").textContent).toBe("false");
448+
449+
act(() => {
450+
fireEvent.click(screen.getByTestId("outer-open"));
451+
});
452+
453+
// Outer opened — inner must follow.
454+
expect(screen.getByTestId("outer-state").textContent).toBe("true");
455+
expect(screen.getByTestId("inner-state").textContent).toBe("true");
456+
});
457+
458+
it("scenario-nested-provider: three-level chain propagates through middle provider", () => {
459+
// Mirrors the real provider stack:
460+
// Provider 1 (user's outer, no isModalDefaultOpen)
461+
// └── Provider 2 (CopilotChat's, no isModalDefaultOpen) — "middle"
462+
// └── Provider 3 (CopilotSidebarView's, explicit isModalDefaultOpen)
463+
//
464+
// Toggling P3 must reach P1 even though P2 has no explicit default.
465+
render(
466+
<CopilotChatConfigurationProvider threadId="p1">
467+
<ModalControls id="p1" />
468+
<CopilotChatConfigurationProvider threadId="p2">
469+
{/* p2 has no isModalDefaultOpen — proxies p1's state */}
470+
<CopilotChatConfigurationProvider
471+
threadId="p3"
472+
isModalDefaultOpen={true}
473+
>
474+
<ModalControls id="p3" />
475+
</CopilotChatConfigurationProvider>
476+
</CopilotChatConfigurationProvider>
477+
</CopilotChatConfigurationProvider>,
478+
);
479+
480+
expect(screen.getByTestId("p1-state").textContent).toBe("true");
481+
expect(screen.getByTestId("p3-state").textContent).toBe("true");
482+
483+
act(() => {
484+
fireEvent.click(screen.getByTestId("p3-close"));
485+
});
486+
487+
expect(screen.getByTestId("p3-state").textContent).toBe("false");
488+
expect(screen.getByTestId("p1-state").textContent).toBe("false");
489+
});
490+
491+
it("scenario-nested-provider: Behavior A still holds after sync fix (no regression)", () => {
492+
// Explicit isModalDefaultOpen on a child must still override the
493+
// parent's current value on initial render — the sync effect must
494+
// not overwrite the child's own initial state.
495+
render(
496+
<CopilotChatConfigurationProvider
497+
threadId="outer"
498+
isModalDefaultOpen={true}
499+
>
500+
<ModalControls id="outer" />
501+
<CopilotChatConfigurationProvider
502+
threadId="inner"
503+
isModalDefaultOpen={false}
504+
>
505+
<ModalControls id="inner" />
506+
</CopilotChatConfigurationProvider>
507+
</CopilotChatConfigurationProvider>,
508+
);
509+
510+
// Inner must start closed despite outer being open.
511+
expect(screen.getByTestId("outer-state").textContent).toBe("true");
512+
expect(screen.getByTestId("inner-state").textContent).toBe("false");
513+
});
333514
});
334515

335516
describe("Nested providers", () => {

0 commit comments

Comments
 (0)