Skip to content
Merged
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
2 changes: 2 additions & 0 deletions apps/app/src/components/sidebar/ThreadRow.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ function renderSplitThreadRow({
afterEach(() => {
cleanup();
resetPluginThreadRowStatusesForTest();
// The layout is tab-scoped, so it lands in both stores (createTabScopedStorage).
window.localStorage.removeItem(SPLIT_LAYOUT_STORAGE_KEY);
window.sessionStorage.removeItem(SPLIT_LAYOUT_STORAGE_KEY);
});

describe("ThreadRow", () => {
Expand Down
45 changes: 43 additions & 2 deletions apps/app/src/lib/browser-storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ interface SyncStringStorage {
) => (() => void) | undefined;
}

interface LocalStorageValueCodec<T> {
interface StoredValueCodec<T> {
parse: (storedValue: string | null, initialValue: T) => T;
serialize: (value: T) => string;
}
Expand All @@ -36,6 +36,13 @@ function getLocalStorage(): Storage | null {
return window.localStorage;
}

function getSessionStorage(): Storage | null {
if (typeof window === "undefined") {
return null;
}
return window.sessionStorage;
}

function subscribeToLocalStorageKey(
key: string,
callback: StoredValueListener,
Expand Down Expand Up @@ -107,8 +114,42 @@ export function createBooleanPreferenceAtom(
);
}

/**
* Storage for state that belongs to one tab rather than to the user, such as
* the split workspace layout.
*
* Reads prefer `sessionStorage` (per tab, survives that tab's reload) and fall
* back to `localStorage` so a newly opened tab still starts from the most
* recent arrangement. Writes go to both. There is deliberately no `storage`
* subscription: that event fires in *other* tabs, so subscribing would make one
* tab adopt another tab's value mid-session — the cross-tab thread bleed in
* issue #873.
*/
export function createTabScopedStorage<T>(
codec: StoredValueCodec<T>,
): SyncStorage<T> {
return {
getItem: (key: string, initialValue: T) => {
// An empty-but-present tab value is a real value (a cleared layout
// serializes to ""), so only a missing key falls back to the seed.
const tabValue = getSessionStorage()?.getItem(key) ?? null;
const storedValue = tabValue ?? getLocalStorage()?.getItem(key) ?? null;
return codec.parse(storedValue, initialValue);
},
setItem: (key: string, value: T) => {
const serialized = codec.serialize(value);
getSessionStorage()?.setItem(key, serialized);
getLocalStorage()?.setItem(key, serialized);
},
removeItem: (key: string) => {
getSessionStorage()?.removeItem(key);
getLocalStorage()?.removeItem(key);
},
};
}

export function createLocalStorageSyncStorage<T>(
codec: LocalStorageValueCodec<T>,
codec: StoredValueCodec<T>,
): SyncStorage<T> {
return {
getItem: (key: string, initialValue: T) =>
Expand Down
77 changes: 77 additions & 0 deletions apps/app/src/lib/split-layout/atoms.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
splitLayoutAtom,
} from "./atoms";
import { countPanes, findPaneByThread, splitPane } from "./ops";
import { serializeSplitLayout, SPLIT_LAYOUT_STORAGE_KEY } from "./persistence";
import type { SplitLayout } from "./types";

function singlePane(threadId: string): SplitLayout {
Expand All @@ -33,6 +34,82 @@ function twoPanes(): SplitLayout {

afterEach(() => {
window.localStorage.clear();
window.sessionStorage.clear();
});

/** A write by another tab: same origin storage, then the cross-tab event. */
function writeFromOtherTab(key: string, value: string): void {
const previous = window.localStorage.getItem(key);
window.localStorage.setItem(key, value);
window.dispatchEvent(
new StorageEvent("storage", {
key,
newValue: value,
oldValue: previous,
storageArea: window.localStorage,
}),
);
}

/**
* A fresh page load: atomWithStorage only re-reads storage when the atom is
* mounted, so subscribe first, exactly like a rendered component does.
*/
function hydrateLayoutOnLoad(): SplitLayout | null {
const store = createStore();
const unsubscribe = store.sub(splitLayoutAtom, () => {});
const layout = store.get(splitLayoutAtom);
unsubscribe();
return layout;
}

describe("tab-scoped workspace state", () => {
it("keeps this tab's panes when another tab opens a different thread", () => {
const store = createStore();
// Mounting is what would install a cross-tab subscription.
const unsubscribe = store.sub(splitLayoutAtom, () => {});
store.set(splitLayoutAtom, singlePane("thread-1"));

writeFromOtherTab(
SPLIT_LAYOUT_STORAGE_KEY,
serializeSplitLayout(singlePane("thread-2")),
);

expect(store.get(splitLayoutAtom)).toEqual(singlePane("thread-1"));
unsubscribe();
});

it("keeps this tab's maximized pane when another tab maximizes a different one", () => {
const store = createStore();
const unsubscribe = store.sub(maximizedPaneIdAtom, () => {});
store.set(splitLayoutAtom, twoPanes());
store.set(maximizedPaneIdAtom, "pane-1");

writeFromOtherTab(MAXIMIZED_PANE_STORAGE_KEY, "pane-2");

expect(store.get(maximizedPaneIdAtom)).toBe("pane-1");
unsubscribe();
});

it("seeds a new tab from the last arrangement, then reloads its own", () => {
// A tab that has never had a layout starts from the shared seed.
window.localStorage.setItem(
SPLIT_LAYOUT_STORAGE_KEY,
serializeSplitLayout(twoPanes()),
);
expect(countPanes(hydrateLayoutOnLoad()!.root)).toBe(2);

// Once this tab owns a layout, its reload restores that one even though
// another tab wrote the shared key afterwards.
const store = createStore();
store.set(splitLayoutAtom, singlePane("thread-3"));
window.localStorage.setItem(
SPLIT_LAYOUT_STORAGE_KEY,
serializeSplitLayout(singlePane("thread-9")),
);

expect(hydrateLayoutOnLoad()).toEqual(singlePane("thread-3"));
});
});

describe("closePanesForThreadsAtom", () => {
Expand Down
36 changes: 21 additions & 15 deletions apps/app/src/lib/split-layout/atoms.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import { atom } from "jotai";
import { atomWithStorage } from "jotai/utils";
import {
createNullableLocalStorageEnumStorage,
createLocalStorageSyncStorage,
type SyncStorage,
} from "@/lib/browser-storage";
import { createTabScopedStorage, type SyncStorage } from "@/lib/browser-storage";
import type { ThreadRoutePathArgs } from "@/lib/route-paths";
import { findPane, listPanes, removePane } from "./ops";
import {
Expand All @@ -15,7 +11,7 @@ import {
import type { SplitLayout } from "./types";

function createSplitLayoutStorage(): SyncStorage<SplitLayout | null> {
return createLocalStorageSyncStorage<SplitLayout | null>({
return createTabScopedStorage<SplitLayout | null>({
// A malformed or stale value deserializes to null, which the split area
// reads as "seed a single pane from the current route".
parse: (storedValue) => deserializeSplitLayout(storedValue),
Expand All @@ -25,10 +21,15 @@ function createSplitLayoutStorage(): SyncStorage<SplitLayout | null> {

/**
* Global split layout, shared across projects like {@link
* sidebarCollapsedAtoms}. Null until the first thread view seeds a single pane
* from the route; persisted through the versioned split-layout codec so reload
* restores the arrangement (the URL's thread claims focus). The focused pane is
* carried inside {@link SplitLayout.focusedPaneId}, not a separate atom.
* sidebarCollapsedAtoms} but scoped to one tab. Null until the first thread
* view seeds a single pane from the route; persisted through the versioned
* split-layout codec so reload restores the arrangement (the URL's thread
* claims focus). The focused pane is carried inside {@link
* SplitLayout.focusedPaneId}, not a separate atom.
*
* Tab-scoped rather than cross-tab synced: which thread sits in which pane is a
* property of the window you are looking at, so a second tab must never adopt
* the first tab's panes (issue #873). See {@link createTabScopedStorage}.
*/
export const splitLayoutAtom = atomWithStorage<SplitLayout | null>(
SPLIT_LAYOUT_STORAGE_KEY,
Expand All @@ -39,20 +40,25 @@ export const splitLayoutAtom = atomWithStorage<SplitLayout | null>(

export const MAXIMIZED_PANE_STORAGE_KEY = "bb.splitLayout.maximizedPaneId";

function isPersistedPaneId(value: string): value is string {
return value.length > 0;
}

/**
* The pane temporarily filling the split workspace. This is persisted beside,
* rather than inside, the versioned split tree so maximizing never rewrites or
* migrates the exact arrangement and sizes it will restore. SplitThreadArea
* validates the id against the hydrated layout and clears stale values.
*
* Tab-scoped for the same reason as {@link splitLayoutAtom}: it names a pane in
* that tab's layout.
*/
export const maximizedPaneIdAtom = atomWithStorage<string | null>(
MAXIMIZED_PANE_STORAGE_KEY,
null,
createNullableLocalStorageEnumStorage(isPersistedPaneId),
createTabScopedStorage<string | null>({
parse: (storedValue, initialValue) =>
storedValue !== null && storedValue.length > 0
? storedValue
: initialValue,
serialize: (value) => value ?? "",
}),
{ getOnInit: true },
);

Expand Down
32 changes: 32 additions & 0 deletions apps/app/src/views/thread-detail/SplitThreadArea.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ afterEach(() => {
resetPluginSlotStoreForTest();
delete window.bbDesktop;
window.localStorage.clear();
window.sessionStorage.clear();
});

describe("SplitThreadArea", () => {
Expand Down Expand Up @@ -1378,6 +1379,37 @@ describe("SplitThreadArea", () => {
).not.toBe(0);
});

it("ignores a layout written by another tab (issue #873)", async () => {
renderSplitArea({ path: threadPath("thr-a") });
expect(await screen.findByTestId("pane-thr-a")).toBeTruthy();

// Another tab selects thr-b: same-origin localStorage write plus the
// `storage` event the browser delivers to every other tab.
const otherTabLayout = serializeSplitLayout({
root: { type: "pane", paneId: "pane-1", content: threadContent("thr-b") },
focusedPaneId: "pane-1",
});
window.localStorage.setItem(SPLIT_LAYOUT_STORAGE_KEY, otherTabLayout);
fireEvent(
window,
new StorageEvent("storage", {
key: SPLIT_LAYOUT_STORAGE_KEY,
newValue: otherTabLayout,
storageArea: window.localStorage,
}),
);

// This tab keeps its own thread and URL; nothing bleeds across tabs.
await waitFor(() => {
expect(screen.getByTestId("location").textContent).toBe(
threadPath("thr-a"),
);
});
expect(screen.queryAllByTestId(/^pane-/)).toHaveLength(1);
expect(screen.getByTestId("pane-thr-a")).toBeTruthy();
expect(screen.queryByTestId("pane-thr-b")).toBeNull();
});

it("falls back to a single pane from the route when persisted state is malformed", async () => {
window.localStorage.setItem(SPLIT_LAYOUT_STORAGE_KEY, "not json");
renderSplitArea({ path: threadPath("thr-a") });
Expand Down
Loading