fix(copilot): keep sidebar mode beside host content instead of overlapping it - #3026
fix(copilot): keep sidebar mode beside host content instead of overlapping it#3026RoyBA wants to merge 5 commits into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libs/copilot/src/hooks/useSidebarResize.ts">
<violation number="1" location="libs/copilot/src/hooks/useSidebarResize.ts:50">
P2: When a host relies on direct-body selectors or body flex/grid layout, opening the sidebar changes its DOM hierarchy and can break the host layout. Wrap a host-provided app root instead, or preserve and explicitly support those body-level layout assumptions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Address review feedback: restore the wrapper's current scroll on close (not the pre-open position), replay the original body child order so hosts whose widget container isn't the last child keep sibling order, and assert body-margin restoration in the close E2E test. Co-Authored-By: GitHub Copilot <noreply@github.com>
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
removeHostWrapper already restores host children to their original order when the widget is the last body child (the mounted case), so the snapshot replay was redundant and could re-insert nodes the host detached while the sidebar was open. Reverts that reorder from review feedback; the live-scroll restore stays. Co-Authored-By: GitHub Copilot <noreply@github.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libs/copilot/src/hooks/useSidebarResize.ts">
<violation number="1">
P2: Removing the `originalOrder` replay drops host sibling order preservation for the non-default case. `removeHostWrapper` re-inserts the wrapper's children immediately before the wrapper (which sits ahead of `#chainlit-copilot`), so after close every host child ends up before the copilot container. If the copilot container is not the last `<body>` child (the exact case the deleted comment documents, e.g. host content appended after the widget is mounted), that content gets permanently moved in front of the copilot on close. The commit labels this replay "redundant", but it is only redundant when the copilot is always the last child; otherwise it fixes a real reorder. Consider keeping an order snapshot (or restoring children with `insertBefore(firstChildNull, ...)` semantics that honor the copilot's original position) so host sibling order survives open/close.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -1,11 +1,20 @@ | |||
| import { useCallback, useEffect, useRef, useState } from 'react'; | |||
| import { | |||
There was a problem hiding this comment.
P2: Removing the originalOrder replay drops host sibling order preservation for the non-default case. removeHostWrapper re-inserts the wrapper's children immediately before the wrapper (which sits ahead of #chainlit-copilot), so after close every host child ends up before the copilot container. If the copilot container is not the last <body> child (the exact case the deleted comment documents, e.g. host content appended after the widget is mounted), that content gets permanently moved in front of the copilot on close. The commit labels this replay "redundant", but it is only redundant when the copilot is always the last child; otherwise it fixes a real reorder. Consider keeping an order snapshot (or restoring children with insertBefore(firstChildNull, ...) semantics that honor the copilot's original position) so host sibling order survives open/close.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/copilot/src/hooks/useSidebarResize.ts, line 184:
<comment>Removing the `originalOrder` replay drops host sibling order preservation for the non-default case. `removeHostWrapper` re-inserts the wrapper's children immediately before the wrapper (which sits ahead of `#chainlit-copilot`), so after close every host child ends up before the copilot container. If the copilot container is not the last `<body>` child (the exact case the deleted comment documents, e.g. host content appended after the widget is mounted), that content gets permanently moved in front of the copilot on close. The commit labels this replay "redundant", but it is only redundant when the copilot is always the last child; otherwise it fixes a real reorder. Consider keeping an order snapshot (or restoring children with `insertBefore(firstChildNull, ...)` semantics that honor the copilot's original position) so host sibling order survives open/close.</comment>
<file context>
@@ -138,10 +138,6 @@ export function useSidebarResize({
const body = document.body;
const { scrollX, scrollY } = window;
- // Snapshot the original body child order — the widget container isn't always the
- // last child, so we restore this exact order on close.
- const originalOrder = Array.from(body.childNodes);
-
// Preserve the host's inline body margins so we can restore them on close.
const previousMargin = {
</file context>
|
Hey, could we get any pic of prior/after the changes ? |
Hosts that arrange content at the body level (e.g. body { display: flex/grid } centering) had their content jump when the sidebar opened, because moving it into the wrapper dropped those rules. Copy the body's layout-affecting computed properties onto the wrapper so reparented content lays out unchanged.
Co-Authored-By: GitHub Copilot <noreply@github.com>
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libs/copilot/src/hooks/useSidebarResize.ts">
<violation number="1" location="libs/copilot/src/hooks/useSidebarResize.ts:52">
P2: When the host defines body padding, this copy applies the padding twice and can make the constrained wrapper overlap the sidebar. Transfer the padding by clearing it on the body while the wrapper is active, then restore it during cleanup.</violation>
<violation number="2" location="libs/copilot/src/hooks/useSidebarResize.ts:67">
P2: When a host uses responsive or fractional body grid tracks, the wrapper freezes the full-width computed tracks instead of reflowing them to its reduced width. Copy the source layout declarations or mirror these properties whenever the viewport or host styles change, rather than keeping one computed snapshot.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| wrapper.id = HOST_WRAPPER_ID; | ||
|
|
||
| const bodyStyle = getComputedStyle(body); | ||
| HOST_LAYOUT_PROPS.forEach((prop) => { |
There was a problem hiding this comment.
P2: When a host uses responsive or fractional body grid tracks, the wrapper freezes the full-width computed tracks instead of reflowing them to its reduced width. Copy the source layout declarations or mirror these properties whenever the viewport or host styles change, rather than keeping one computed snapshot.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/copilot/src/hooks/useSidebarResize.ts, line 67:
<comment>When a host uses responsive or fractional body grid tracks, the wrapper freezes the full-width computed tracks instead of reflowing them to its reduced width. Copy the source layout declarations or mirror these properties whenever the viewport or host styles change, rather than keeping one computed snapshot.</comment>
<file context>
@@ -40,6 +63,11 @@ function createHostWrapper(): HTMLElement {
wrapper.id = HOST_WRAPPER_ID;
+ const bodyStyle = getComputedStyle(body);
+ HOST_LAYOUT_PROPS.forEach((prop) => {
+ wrapper.style.setProperty(prop, bodyStyle.getPropertyValue(prop));
+ });
</file context>
| 'grid-auto-rows', | ||
| 'row-gap', | ||
| 'column-gap', | ||
| 'padding', |
There was a problem hiding this comment.
P2: When the host defines body padding, this copy applies the padding twice and can make the constrained wrapper overlap the sidebar. Transfer the padding by clearing it on the body while the wrapper is active, then restore it during cleanup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/copilot/src/hooks/useSidebarResize.ts, line 52:
<comment>When the host defines body padding, this copy applies the padding twice and can make the constrained wrapper overlap the sidebar. Transfer the padding by clearing it on the body while the wrapper is active, then restore it during cleanup.</comment>
<file context>
@@ -30,6 +30,29 @@ function getHostWrapper(): HTMLElement | null {
+ 'grid-auto-rows',
+ 'row-gap',
+ 'column-gap',
+ 'padding',
+ 'text-align'
+];
</file context>
2 participants
Summary
In the Copilot widget's sidebar display mode, the fixed side panel could
render on top of and hide host-page UI on some sites. This replaces the old
"push the host via a
document.bodymargin" approach with a wrapper thatconstrains host content so the sidebar sits beside it. Floating mode and the
standalone Chainlit app are unaffected.
Root cause
Sidebar mode renders a viewport-
fixedpanel (fixed top-0 right-0 h-full z-50).The previous
useSidebarResizeonly setdocument.body.style.marginRightto makeroom. A body margin only shrinks a host whose width derives from the
<body>content box. On viewport-anchored hosts —
position: fixed/absolute inset: 0,width: 100vw,overflow:hidden+ inner scroller, orbody { margin: 0 !important }—the margin has no effect, so the fixed sidebar overlaps host elements. That's the
"works on some hosts, overlaps on others" report.
Fix
While the sidebar is open,
useSidebarResizenow:<body>child except the widget container (#chainlit-copilot)into a controlled
#chainlit-copilot-host-wrapper(body's first child), andrestores the original DOM on close / mode-switch / unmount.
width: calc(100vw - {sidebarWidth}px),height: 100vh,box-sizing: border-box,overflow-x: hidden; overflow-y: auto, width transition.transform: translateZ(0)on the wrapper — this makes it the containingblock for
position: fixed/absolutedescendants, forcing viewport-anchoredhost layouts to reflow within the reduced width instead of sliding under the sidebar.
carries the host scroll position onto the wrapper (restored on close). Runs as a
useLayoutEffectto avoid a flash of unconstrained content. Drag-resize nowtransitions the wrapper rather than
body.Known tradeoffs
Establishing a containing block changes
position: fixedsemantics for hostelements (they become wrapper-relative). Hosts with hard-coded
100vw/fixedwidths won't reflow and are clipped to the reduced width (horizontal overflow is
hidden by design). A future opt-in config to target a host-provided app root
(instead of wrapping
body) is a reasonable follow-up if any host regresses.Testing
pnpm lint,pnpm format-checkpass on both files.pnpm type-checkclean;pnpm --filter @chainlit/copilot buildsucceeds.(exists,
width === 'calc(100vw - 400px)',wrapperWidth + sidebarWidth ≈ innerWidth, removed on close/unmount). Runs in CI.fixed inset:0SPA, and a100vwhost; with host zoom + non-default root font-size; light/dark; plusdrag-resize and open → close → unmount (host DOM, margins, and scroll fully restore).