Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 2 additions & 26 deletions packages/tui/src/routes/session/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
batch,
createContext,
createEffect,
createMemo,
Expand Down Expand Up @@ -247,7 +246,6 @@ export function Session() {

const dimensions = useTerminalDimensions()
const [sidebar, setSidebar] = kv.signal<"auto" | "hide">("sidebar", "auto")
const [sidebarOpen, setSidebarOpen] = createSignal(false)
const [conceal, setConceal] = createSignal(true)
const thinking = useThinkingMode()
const thinkingMode = thinking.mode
Expand All @@ -263,7 +261,6 @@ export function Session() {
const wide = createMemo(() => dimensions().width > 120)
const sidebarVisible = createMemo(() => {
if (session()?.parentID) return false
if (sidebarOpen()) return true
if (sidebar() === "auto" && wide()) return true
return false
})
Expand Down Expand Up @@ -668,11 +665,7 @@ export function Session() {
value: "session.sidebar.toggle",
category: "Session",
run: () => {
batch(() => {
const isVisible = sidebarVisible()
setSidebar(() => (isVisible ? "hide" : "auto"))
setSidebarOpen(!isVisible)
})
setSidebar((prev) => (prev === "auto" ? "hide" : "auto"))
dialog.clear()
},
},
Expand Down Expand Up @@ -1322,24 +1315,7 @@ export function Session() {
<Toast />
</box>
<Show when={sidebarVisible()}>
<Switch>
<Match when={wide()}>
<Sidebar sessionID={route.sessionID} />
</Match>
<Match when={!wide()}>
<box
position="absolute"
top={0}
left={0}
right={0}
bottom={0}
alignItems="flex-end"
backgroundColor={RGBA.fromInts(0, 0, 0, 70)}
>
<Sidebar sessionID={route.sessionID} />
</box>
</Match>
</Switch>
<Sidebar sessionID={route.sessionID} />
</Show>
</box>
</context.Provider>
Expand Down
146 changes: 146 additions & 0 deletions packages/tui/test/session/sidebar.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { createMemo, createRoot, createSignal } from "solid-js"
import { expect, test } from "bun:test"

/**
* The sidebar visibility logic in the session component determines
* whether to show the sidebar based on:
* 1. sidebar mode: "auto" (show when screen is wide enough) or "hide" (always hide)
* 2. screen width: wide() returns true when terminal width > 120
*
* Previously there was a `sidebarOpen` signal that overrode the width check,
* causing the sidebar to force-show when toggled on via command palette
* even on narrow screens (windowed mode). This was the bug.
*
* These tests validate the corrected behavior.
*/

test("sidebar hidden when mode is auto and screen is not wide", () => {
createRoot(() => {
const [sidebar] = createSignal<"auto" | "hide">("auto")
const [wide] = createSignal(false)

const sidebarVisible = createMemo(() => {
if (sidebar() === "auto" && wide()) return true
return false
})

expect(sidebarVisible()).toBe(false)
})
})

test("sidebar visible when mode is auto and screen is wide", () => {
createRoot(() => {
const [sidebar] = createSignal<"auto" | "hide">("auto")
const [wide] = createSignal(true)

const sidebarVisible = createMemo(() => {
if (sidebar() === "auto" && wide()) return true
return false
})

expect(sidebarVisible()).toBe(true)
})
})

test("sidebar hidden when mode is hide regardless of screen width", () => {
createRoot(() => {
const [sidebar] = createSignal<"auto" | "hide">("hide")
const [wide, setWide] = createSignal(true)

const sidebarVisible = createMemo(() => {
if (sidebar() === "auto" && wide()) return true
return false
})

expect(sidebarVisible()).toBe(false)

// Even when wide, sidebar is hidden because mode is "hide"
setWide(true)
expect(sidebarVisible()).toBe(false)
})
})

test("sidebar visibility reacts to width changes in auto mode", () => {
createRoot(() => {
const [sidebar] = createSignal<"auto" | "hide">("auto")
const [wide, setWide] = createSignal(false)

const sidebarVisible = createMemo(() => {
if (sidebar() === "auto" && wide()) return true
return false
})

expect(sidebarVisible()).toBe(false)

// Resize from narrow to wide
setWide(true)
expect(sidebarVisible()).toBe(true)

// Resize from wide back to narrow
setWide(false)
expect(sidebarVisible()).toBe(false)
})
})

test("toggling sidebar switches between auto and hide modes", () => {
createRoot(() => {
const [sidebar, setSidebar] = createSignal<"auto" | "hide">("auto")

// Toggle: auto -> hide
setSidebar("hide")
expect(sidebar()).toBe("hide")

// Toggle: hide -> auto
setSidebar("auto")
expect(sidebar()).toBe("auto")
})
})

test("toggling sidebar on with narrow window does NOT force visibility (regression test)", () => {
createRoot(() => {
const [sidebar, setSidebar] = createSignal<"auto" | "hide">("auto")
const [wide] = createSignal(false)

const sidebarVisible = createMemo(() => {
if (sidebar() === "auto" && wide()) return true
return false
})

// Initially hidden
expect(sidebarVisible()).toBe(false)

// Toggle: user toggles "on" (set mode to auto), but screen is still narrow
// The old bug: sidebarOpen would force visibility here
setSidebar("auto")

// The fix: sidebar should still be hidden because screen is not wide
expect(sidebarVisible()).toBe(false)
})
})

test("sidebar toggle cycle with width changes works correctly", () => {
createRoot(() => {
const [sidebar, setSidebar] = createSignal<"auto" | "hide">("hide")
const [wide, setWide] = createSignal(false)

const sidebarVisible = createMemo(() => {
if (sidebar() === "auto" && wide()) return true
return false
})

// 1. Start: hide, narrow -> not visible
expect(sidebarVisible()).toBe(false)

// 2. User enables sidebar, but still narrow -> should not force show
setSidebar("auto")
expect(sidebarVisible()).toBe(false)

// 3. User makes window wide -> sidebar becomes visible
setWide(true)
expect(sidebarVisible()).toBe(true)

// 4. User makes window narrow again -> sidebar hides
setWide(false)
expect(sidebarVisible()).toBe(false)
})
})
Loading