fix(app): unmount hidden session panes (#35628)

This commit is contained in:
Luke Parker 2026-07-09 09:44:02 +10:00 committed by GitHub
commit d8a57ee15c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 286 additions and 306 deletions

View file

@ -168,8 +168,8 @@ test("keeps the review tree and terminal sized when both panels are open", async
await expectTree(page, 2_773, "action.yml") await expectTree(page, 2_773, "action.yml")
await page.getByRole("button", { name: "Toggle file tree" }).click() await page.getByRole("button", { name: "Toggle file tree" }).click()
await expect(page.locator('[data-slot="session-review-v2-sidebar"]')).toHaveAttribute("aria-hidden", "true") await expect(page.locator('[data-slot="session-review-v2-sidebar"]')).toHaveCount(0)
await expect(page.locator('#review-panel [data-component="file-tree-v2"]')).toHaveCount(1) await expect(page.locator('#review-panel [data-component="file-tree-v2"]')).toHaveCount(0)
await page.getByRole("button", { name: "Toggle file tree" }).click() await page.getByRole("button", { name: "Toggle file tree" }).click()
await expectTree(page, 2_773, "action.yml") await expectTree(page, 2_773, "action.yml")
@ -181,8 +181,7 @@ test("keeps the review tree and terminal sized when both panels are open", async
await expectTree(page, 2_773, "action.yml") await expectTree(page, 2_773, "action.yml")
await page.getByRole("button", { name: "Toggle review" }).click() await page.getByRole("button", { name: "Toggle review" }).click()
await expect(page.locator("#review-panel")).toHaveAttribute("aria-hidden", "true") await expect(page.locator("#review-panel")).toHaveCount(0)
await expect(page.locator('#review-panel [data-component="file-tree-v2"]')).toHaveCount(1)
await page.getByRole("button", { name: "Toggle review" }).click() await page.getByRole("button", { name: "Toggle review" }).click()
await expectTree(page, 2_773, "action.yml") await expectTree(page, 2_773, "action.yml")
await page.setViewportSize({ width: 1_000, height: 700 }) await page.setViewportSize({ width: 1_000, height: 700 })

View file

@ -2198,7 +2198,7 @@ export default function Page() {
</Show> </Show>
</div> </div>
<Show when={!newSessionDesign()}> <Show when={!newSessionDesign() && desktopSidePanelOpen()}>
<SessionSidePanel <SessionSidePanel
canReview={canReview} canReview={canReview}
diffs={reviewDiffs} diffs={reviewDiffs}
@ -2217,13 +2217,8 @@ export default function Page() {
<Show when={newSessionDesign()}> <Show when={newSessionDesign()}>
<Show when={isDesktop() ? desktopV2PanelLayout().visible : terminalOpen()}> <Show when={isDesktop() ? desktopV2PanelLayout().visible : terminalOpen()}>
<div class="min-w-0 h-full flex flex-1 flex-col"> <div class="min-w-0 h-full flex flex-1 flex-col">
<Show when={isDesktop()}> <Show when={isDesktop() && (desktopV2ReviewOpen() || desktopFileTreeOpen())}>
<div <div class="min-h-0 flex-1">
classList={{
"min-h-0 flex-1": desktopV2ReviewOpen() || desktopFileTreeOpen(),
"size-0 shrink-0 overflow-hidden": !(desktopV2ReviewOpen() || desktopFileTreeOpen()),
}}
>
<SessionSidePanel <SessionSidePanel
canReview={canReview} canReview={canReview}
diffs={reviewDiffs} diffs={reviewDiffs}

View file

@ -81,7 +81,6 @@ export function SessionSidePanel(props: {
}), }),
) )
const open = createMemo(() => reviewOpen() || fileOpen()) const open = createMemo(() => reviewOpen() || fileOpen())
const rendered = createMemo<boolean>((previous) => previous || open(), false)
const reviewTab = createMemo(() => isDesktop()) const reviewTab = createMemo(() => isDesktop())
const panelWidth = createMemo(() => { const panelWidth = createMemo(() => {
if (!open()) return "0px" if (!open()) return "0px"
@ -164,10 +163,6 @@ export function SessionSidePanel(props: {
const openedTabs = tabState.openedTabs const openedTabs = tabState.openedTabs
const activeTab = tabState.activeTab const activeTab = tabState.activeTab
const activeFileTab = tabState.activeFileTab const activeFileTab = tabState.activeFileTab
const reviewContentRendered = createMemo<boolean>(
(previous) => previous || (reviewOpen() && activeTab() === "review"),
false,
)
const fileTreeTab = () => layout.fileTree.tab() const fileTreeTab = () => layout.fileTree.tab()
@ -272,233 +267,220 @@ export function SessionSidePanel(props: {
}} }}
style={{ width: panelWidth() }} style={{ width: panelWidth() }}
> >
<Show when={rendered()}> <Show when={open()}>
<div <div
class="size-full flex" class="size-full flex"
classList={{ classList={{
"border-l border-border-weaker-base": !settings.general.newLayoutDesigns(), "border-l border-border-weaker-base": !settings.general.newLayoutDesigns(),
}} }}
> >
<div <Show when={reviewOpen()}>
aria-hidden={!reviewOpen()} <div class="relative min-w-0 h-full flex-1 overflow-hidden bg-background-base">
inert={!reviewOpen()} <div class="size-full min-w-0 h-full bg-background-base">
class="relative min-w-0 h-full flex-1 overflow-hidden bg-background-base" <DragDropProvider
classList={{ onDragStart={handleDragStart}
"pointer-events-none": !reviewOpen(), onDragEnd={handleDragEnd}
}} onDragOver={handleDragOver}
> collisionDetector={closestCenter}
<div class="size-full min-w-0 h-full bg-background-base"> >
<DragDropProvider <DragDropSensors />
onDragStart={handleDragStart} <ConstrainDragYAxis />
onDragEnd={handleDragEnd} <Tabs value={activeTab()} onChange={activateTab}>
onDragOver={handleDragOver} <div class="sticky top-0 shrink-0 flex">
collisionDetector={closestCenter} <Tabs.List
> ref={(el: HTMLDivElement) => {
<DragDropSensors /> const stop = createFileTabListSync({ el, contextOpen })
<ConstrainDragYAxis /> onCleanup(stop)
<Tabs value={activeTab()} onChange={activateTab}> }}
<div class="sticky top-0 shrink-0 flex"> >
<Tabs.List <Show when={reviewTab() && props.canReview()}>
ref={(el: HTMLDivElement) => { <Tabs.Trigger
const stop = createFileTabListSync({ el, contextOpen }) value="review"
onCleanup(stop) id={reviewTabID}
}} aria-controls={activeTab() === "review" ? reviewTabPanelID : undefined}
> >
<Show when={reviewTab() && props.canReview()}> <div class="flex items-center gap-1.5">
<Tabs.Trigger <div>{language.t("session.tab.review")}</div>
value="review" <Show when={props.hasReview()}>
id={reviewTabID} <div>{props.reviewCount()}</div>
aria-controls={activeTab() === "review" ? reviewTabPanelID : undefined} </Show>
> </div>
<div class="flex items-center gap-1.5"> </Tabs.Trigger>
<div>{language.t("session.tab.review")}</div> </Show>
<Show when={props.hasReview()}> <Show when={contextOpen()}>
<div>{props.reviewCount()}</div> <Tabs.Trigger
</Show> value="context"
</div> closeButton={
</Tabs.Trigger> <TooltipKeybind
</Show> title={language.t("common.closeTab")}
<Show when={contextOpen()}> keybind={command.keybind("tab.close")}
<Tabs.Trigger placement="bottom"
value="context" gutter={10}
closeButton={
<TooltipKeybind
title={language.t("common.closeTab")}
keybind={command.keybind("tab.close")}
placement="bottom"
gutter={10}
>
<IconButton
icon="close-small"
variant="ghost"
class="h-5 w-5"
onClick={() => tabs().close("context")}
aria-label={language.t("common.closeTab")}
/>
</TooltipKeybind>
}
hideCloseButton
onMiddleClick={() => tabs().close("context")}
>
<div class="flex items-center gap-2">
<SessionContextUsage variant="indicator" />
<div>{language.t("session.tab.context")}</div>
</div>
</Tabs.Trigger>
</Show>
<SortableProvider ids={openedTabs()}>
<For each={panelTabs()}>
{(tab) => (
<Show
when={tab === SESSION_OPEN_FILE_TAB}
fallback={
<SortableTab
tab={tab}
temporary={temporaryTab() === tab}
onTabClose={tabs().close}
onTabDoubleClick={temporaryTab() === tab ? openTab : undefined}
/>
}
>
<Tabs.Trigger
value={SESSION_OPEN_FILE_TAB}
closeButton={
<TooltipKeybind
title={language.t("common.closeTab")}
keybind={command.keybind("tab.close")}
placement="bottom"
gutter={10}
>
<IconButton
icon="close-small"
variant="ghost"
class="h-5 w-5"
onClick={() => tabs().close(SESSION_OPEN_FILE_TAB)}
aria-label={language.t("common.closeTab")}
/>
</TooltipKeybind>
}
hideCloseButton
onMiddleClick={() => tabs().close(SESSION_OPEN_FILE_TAB)}
> >
<div class="flex items-center gap-1.5 italic"> <IconButton
<Icon name="open-file" size="small" /> icon="close-small"
<span>{language.t("command.file.open")}</span> variant="ghost"
</div> class="h-5 w-5"
</Tabs.Trigger> onClick={() => tabs().close("context")}
</Show> aria-label={language.t("common.closeTab")}
)} />
</For> </TooltipKeybind>
</SortableProvider> }
<div class="bg-background-stronger h-full shrink-0 sticky right-0 z-10 flex items-center justify-center pr-3"> hideCloseButton
<TooltipKeybind onMiddleClick={() => tabs().close("context")}
title={language.t("command.file.open")} >
keybind={command.keybind("file.open")} <div class="flex items-center gap-2">
class="flex items-center" <SessionContextUsage variant="indicator" />
> <div>{language.t("session.tab.context")}</div>
<IconButton </div>
icon="plus-small" </Tabs.Trigger>
variant="ghost" </Show>
iconSize="large" <SortableProvider ids={openedTabs()}>
class="!rounded-md" <For each={panelTabs()}>
onClick={() => { {(tab) => (
if (props.fileBrowserState) { <Show
openFileBrowser() when={tab === SESSION_OPEN_FILE_TAB}
return fallback={
} <SortableTab
void import("@/components/dialog-select-file").then((x) => { tab={tab}
dialog.show(() => <x.DialogSelectFile mode="files" onOpenFile={showAllFiles} />) temporary={temporaryTab() === tab}
}) onTabClose={tabs().close}
}} onTabDoubleClick={temporaryTab() === tab ? openTab : undefined}
aria-label={language.t("command.file.open")} />
/> }
</TooltipKeybind> >
</div> <Tabs.Trigger
</Tabs.List> value={SESSION_OPEN_FILE_TAB}
</div> closeButton={
<TooltipKeybind
<Show when={reviewTab() && props.canReview() && reviewContentRendered()}> title={language.t("common.closeTab")}
<div keybind={command.keybind("tab.close")}
id={reviewTabPanelID} placement="bottom"
role="tabpanel" gutter={10}
aria-labelledby={reviewTabID} >
aria-hidden={activeTab() !== "review"} <IconButton
inert={activeTab() !== "review"} icon="close-small"
tabIndex={props.reviewHasFocusableContent() ? undefined : 0} variant="ghost"
data-slot="tabs-content" class="h-5 w-5"
class="flex flex-col h-full overflow-hidden contain-strict" onClick={() => tabs().close(SESSION_OPEN_FILE_TAB)}
classList={{ hidden: activeTab() !== "review" }} aria-label={language.t("common.closeTab")}
> />
{props.reviewPanel()} </TooltipKeybind>
</div> }
</Show> hideCloseButton
onMiddleClick={() => tabs().close(SESSION_OPEN_FILE_TAB)}
<Tabs.Content value="empty" class="flex flex-col h-full overflow-hidden contain-strict"> >
<Show when={activeTab() === "empty"}> <div class="flex items-center gap-1.5 italic">
<div class="relative pt-2 flex-1 min-h-0 overflow-hidden"> <Icon name="open-file" size="small" />
<div class="h-full px-6 pb-42 -mt-4 flex flex-col items-center justify-center text-center gap-6"> <span>{language.t("command.file.open")}</span>
<Mark class="w-14 opacity-10" /> </div>
<div class="text-14-regular text-text-weak max-w-56"> </Tabs.Trigger>
{language.t("session.files.selectToOpen")} </Show>
</div> )}
</For>
</SortableProvider>
<div class="bg-background-stronger h-full shrink-0 sticky right-0 z-10 flex items-center justify-center pr-3">
<TooltipKeybind
title={language.t("command.file.open")}
keybind={command.keybind("file.open")}
class="flex items-center"
>
<IconButton
icon="plus-small"
variant="ghost"
iconSize="large"
class="!rounded-md"
onClick={() => {
if (props.fileBrowserState) {
openFileBrowser()
return
}
void import("@/components/dialog-select-file").then((x) => {
dialog.show(() => <x.DialogSelectFile mode="files" onOpenFile={showAllFiles} />)
})
}}
aria-label={language.t("command.file.open")}
/>
</TooltipKeybind>
</div> </div>
</Tabs.List>
</div>
<Show when={reviewTab() && props.canReview() && activeTab() === "review"}>
<div
id={reviewTabPanelID}
role="tabpanel"
aria-labelledby={reviewTabID}
tabIndex={props.reviewHasFocusableContent() ? undefined : 0}
data-slot="tabs-content"
class="flex flex-col h-full overflow-hidden contain-strict"
>
{props.reviewPanel()}
</div> </div>
</Show> </Show>
</Tabs.Content>
<Show when={contextOpen()}> <Show when={activeTab() === "empty"}>
<Tabs.Content value="context" class="flex flex-col h-full overflow-hidden contain-strict"> <Tabs.Content value="empty" class="flex flex-col h-full overflow-hidden contain-strict">
<Show when={activeTab() === "context"}> <div class="relative pt-2 flex-1 min-h-0 overflow-hidden">
<div class="h-full px-6 pb-42 -mt-4 flex flex-col items-center justify-center text-center gap-6">
<Mark class="w-14 opacity-10" />
<div class="text-14-regular text-text-weak max-w-56">
{language.t("session.files.selectToOpen")}
</div>
</div>
</div>
</Tabs.Content>
</Show>
<Show when={activeTab() === "context"}>
<Tabs.Content value="context" class="flex flex-col h-full overflow-hidden contain-strict">
<div class="relative pt-2 flex-1 min-h-0 overflow-hidden"> <div class="relative pt-2 flex-1 min-h-0 overflow-hidden">
<SessionContextTab /> <SessionContextTab />
</div> </div>
</Show> </Tabs.Content>
</Tabs.Content> </Show>
</Show>
<Show when={browserTab()}> <Show when={browserTab()}>
<SessionFileBrowserTab <SessionFileBrowserTab
tab={browserTab()!} tab={browserTab()!}
placeholder={browserTab() === SESSION_OPEN_FILE_TAB} placeholder={browserTab() === SESSION_OPEN_FILE_TAB}
active={file.pathFromTab(browserTab()!)} active={file.pathFromTab(browserTab()!)}
kinds={browserKinds()} kinds={browserKinds()}
state={props.fileBrowserState!} state={props.fileBrowserState!}
onSelect={(path) => previewTab(file.tab(path))} onSelect={(path) => previewTab(file.tab(path))}
onSelectPermanent={(path) => openTab(file.tab(path))} onSelectPermanent={(path) => openTab(file.tab(path))}
filterRef={(element) => (fileFilter = element)} filterRef={(element) => (fileFilter = element)}
/> />
</Show> </Show>
<Show when={!props.fileBrowserState && activeFileTab()} keyed> <Show when={!props.fileBrowserState && activeFileTab()} keyed>
{(tab) => <FileTabContent tab={tab} />} {(tab) => <FileTabContent tab={tab} />}
</Show> </Show>
</Tabs> </Tabs>
<DragOverlay> <DragOverlay>
<Show when={store.activeDraggable} keyed> <Show when={store.activeDraggable} keyed>
{(tab) => { {(tab) => {
const path = file.pathFromTab(tab) const path = file.pathFromTab(tab)
return ( return (
<div data-component="tabs-drag-preview"> <div data-component="tabs-drag-preview">
<Show when={path}> <Show when={path}>
{(p) => <FileVisual active path={p()} temporary={temporaryTab() === tab} />} {(p) => <FileVisual active path={p()} temporary={temporaryTab() === tab} />}
</Show> </Show>
</div> </div>
) )
}} }}
</Show> </Show>
</DragOverlay> </DragOverlay>
</DragDropProvider> </DragDropProvider>
</div>
</div> </div>
</div> </Show>
<Show when={shown()}> <Show when={fileOpen()}>
<div <div
id="file-tree-panel" id="file-tree-panel"
aria-hidden={!fileOpen()}
inert={!fileOpen()}
class="relative min-w-0 h-full shrink-0 overflow-hidden" class="relative min-w-0 h-full shrink-0 overflow-hidden"
classList={{ classList={{
"pointer-events-none": !fileOpen(),
"transition-[width] duration-200 ease-[cubic-bezier(0.22,1,0.36,1)] will-change-[width] motion-reduce:transition-none": "transition-[width] duration-200 ease-[cubic-bezier(0.22,1,0.36,1)] will-change-[width] motion-reduce:transition-none":
!props.size.active(), !props.size.active(),
}} }}
@ -526,45 +508,49 @@ export function SessionSidePanel(props: {
{language.t("session.files.all")} {language.t("session.files.all")}
</Tabs.Trigger> </Tabs.Trigger>
</Tabs.List> </Tabs.List>
<Tabs.Content value="changes" class="bg-background-stronger px-3 py-0"> <Show when={fileTreeTab() === "changes"}>
<Switch> <Tabs.Content value="changes" class="bg-background-stronger px-3 py-0">
<Match when={props.hasReview() || !props.diffsReady()}> <Switch>
<Show <Match when={props.hasReview() || !props.diffsReady()}>
when={props.diffsReady()} <Show
fallback={ when={props.diffsReady()}
<div class="px-2 py-2 text-12-regular text-text-weak"> fallback={
{language.t("common.loading")} <div class="px-2 py-2 text-12-regular text-text-weak">
{language.t("common.loading.ellipsis")} {language.t("common.loading")}
</div> {language.t("common.loading.ellipsis")}
} </div>
> }
>
<FileTree
path=""
class="pt-3"
allowed={diffFiles()}
kinds={kinds()}
draggable={false}
active={props.activeDiff}
onFileClick={(node) => props.focusReviewDiff(node.path)}
/>
</Show>
</Match>
</Switch>
</Tabs.Content>
</Show>
<Show when={fileTreeTab() === "all"}>
<Tabs.Content value="all" class="bg-background-stronger px-3 py-0">
<Switch>
<Match when={nofiles()}>{empty(language.t("session.files.empty"))}</Match>
<Match when={true}>
<FileTree <FileTree
path="" path=""
class="pt-3" class="pt-3"
allowed={diffFiles()} modified={diffFiles()}
kinds={kinds()} kinds={kinds()}
draggable={false} onFileClick={(node) => openTab(file.tab(node.path))}
active={props.activeDiff}
onFileClick={(node) => props.focusReviewDiff(node.path)}
/> />
</Show> </Match>
</Match> </Switch>
</Switch> </Tabs.Content>
</Tabs.Content> </Show>
<Tabs.Content value="all" class="bg-background-stronger px-3 py-0">
<Switch>
<Match when={nofiles()}>{empty(language.t("session.files.empty"))}</Match>
<Match when={true}>
<FileTree
path=""
class="pt-3"
modified={diffFiles()}
kinds={kinds()}
onFileClick={(node) => openTab(file.tab(node.path))}
/>
</Match>
</Switch>
</Tabs.Content>
</Tabs> </Tabs>
</div> </div>
<Show when={fileOpen()}> <Show when={fileOpen()}>

View file

@ -74,62 +74,62 @@ export function SessionReviewV2Sidebar(props: SessionReviewV2SidebarProps) {
return ( return (
<div data-component="session-review-v2-sidebar-root"> <div data-component="session-review-v2-sidebar-root">
<aside <Show when={props.open}>
data-slot="session-review-v2-sidebar" <aside
data-resizing={resizing() ? "" : undefined} data-slot="session-review-v2-sidebar"
aria-hidden={!props.open} data-resizing={resizing() ? "" : undefined}
inert={!props.open} style={{ width: `${width()}px` }}
style={{ width: props.open ? `${width()}px` : "0px" }}
>
<div data-slot="session-review-v2-sidebar-header">
<div data-slot="session-review-v2-sidebar-title">{props.title}</div>
{props.stats}
</div>
<div data-slot="session-review-v2-sidebar-filter">
<TextInputV2
type="search"
value={props.filter}
onInput={(event) => props.onFilterChange(event.currentTarget.value)}
onKeyDown={props.onFilterKeyDown}
autofocus={props.filterAutofocus}
ref={props.filterRef}
role={props.filterControls ? "combobox" : undefined}
aria-autocomplete={props.filterControls ? "list" : undefined}
aria-controls={props.filterControls}
aria-activedescendant={props.filterActiveDescendant}
aria-expanded={props.filterControls ? props.filterExpanded : undefined}
showClearButton={props.filter.length > 0}
clearLabel={i18n.t("ui.list.clearFilter")}
onClearClick={() => props.onFilterChange("")}
placeholder={i18n.t("ui.sessionReviewV2.filterFiles")}
aria-label={i18n.t("ui.sessionReviewV2.filterFiles")}
leadingIcon={
<svg
width="14"
height="14"
viewBox="0 0 14 14"
fill="none"
xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
>
<path
d="M12.25 12.25L10.0625 10.0625M11.0833 6.41667C11.0833 8.994 8.994 11.0833 6.41667 11.0833C3.83934 11.0833 1.75 8.994 1.75 6.41667C1.75 3.83934 3.83934 1.75 6.41667 1.75C8.994 1.75 11.0833 3.83934 11.0833 6.41667Z"
stroke="currentColor"
stroke-linecap="square"
/>
</svg>
}
/>
</div>
<ScrollView
data-slot="session-review-v2-sidebar-tree"
class="group/file-tree-v2"
thumbVisibility="scroll"
viewportRef={props.viewportRef}
> >
{props.children} <div data-slot="session-review-v2-sidebar-header">
</ScrollView> <div data-slot="session-review-v2-sidebar-title">{props.title}</div>
</aside> {props.stats}
</div>
<div data-slot="session-review-v2-sidebar-filter">
<TextInputV2
type="search"
value={props.filter}
onInput={(event) => props.onFilterChange(event.currentTarget.value)}
onKeyDown={props.onFilterKeyDown}
autofocus={props.filterAutofocus}
ref={props.filterRef}
role={props.filterControls ? "combobox" : undefined}
aria-autocomplete={props.filterControls ? "list" : undefined}
aria-controls={props.filterControls}
aria-activedescendant={props.filterActiveDescendant}
aria-expanded={props.filterControls ? props.filterExpanded : undefined}
showClearButton={props.filter.length > 0}
clearLabel={i18n.t("ui.list.clearFilter")}
onClearClick={() => props.onFilterChange("")}
placeholder={i18n.t("ui.sessionReviewV2.filterFiles")}
aria-label={i18n.t("ui.sessionReviewV2.filterFiles")}
leadingIcon={
<svg
width="14"
height="14"
viewBox="0 0 14 14"
fill="none"
xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
>
<path
d="M12.25 12.25L10.0625 10.0625M11.0833 6.41667C11.0833 8.994 8.994 11.0833 6.41667 11.0833C3.83934 11.0833 1.75 8.994 1.75 6.41667C1.75 3.83934 3.83934 1.75 6.41667 1.75C8.994 1.75 11.0833 3.83934 11.0833 6.41667Z"
stroke="currentColor"
stroke-linecap="square"
/>
</svg>
}
/>
</div>
<ScrollView
data-slot="session-review-v2-sidebar-tree"
class="group/file-tree-v2"
thumbVisibility="scroll"
viewportRef={props.viewportRef}
>
{props.children}
</ScrollView>
</aside>
</Show>
<Show when={props.open && props.onWidthChange}> <Show when={props.open && props.onWidthChange}>
<div data-slot="session-review-v2-sidebar-resize" onPointerDown={() => setResizing(true)}> <div data-slot="session-review-v2-sidebar-resize" onPointerDown={() => setResizing(true)}>
<ResizeHandle <ResizeHandle