From 16b297f488441fb10795b3b2143b8db60ce78930 Mon Sep 17 00:00:00 2001 From: Reece Date: Tue, 1 Sep 2026 14:34:04 +0100 Subject: [PATCH] fix(editor): library controls in the bar's own row, and stop a remount reopening it The registration hook only renders into the retractable tool row, so moving the library's controls there gave it a second bar rather than putting them in the one that was already on screen. A "bar" section renders in the bar's own row instead, inside the container the shared global controls use - so these take the same icon treatment and are measured by the same reflow. That surfaced a worse bug. Deriving the view from the path was keyed on a ref, so a HomePage remount - a Suspense boundary resolving, a login bounce - read the unchanged path as a fresh arrival and re-imposed the library over a view the user had just chosen. Reading mode looked engaged with the file manager still on screen: the bug the last commit set out to fix, through a door its own comment warned about. Keyed on the path value at module scope now, which a remount does not reset and a page load does. The spec that caught it failed only under parallel load; it now waits for the library to be on screen rather than for its path, which is a render ahead. --- .../useFileLibraryWorkbenchBarButtons.tsx | 8 +++---- .../core/components/shared/WorkbenchBar.tsx | 21 +++++++++++++++++++ frontend/editor/src/core/pages/HomePage.tsx | 14 ++++++++++--- .../tests/stubbed/file-library-view.spec.ts | 5 +++++ .../editor/src/core/types/workbenchBar.ts | 9 +++++++- 5 files changed, 49 insertions(+), 8 deletions(-) diff --git a/frontend/editor/src/core/components/filesPage/useFileLibraryWorkbenchBarButtons.tsx b/frontend/editor/src/core/components/filesPage/useFileLibraryWorkbenchBarButtons.tsx index f33e3f5093..68670840e1 100644 --- a/frontend/editor/src/core/components/filesPage/useFileLibraryWorkbenchBarButtons.tsx +++ b/frontend/editor/src/core/components/filesPage/useFileLibraryWorkbenchBarButtons.tsx @@ -45,7 +45,7 @@ export function useFileLibraryWorkbenchBarButtons({ refreshDisabledReason ?? t("filesPage.refresh", "Refresh from server"), ariaLabel: t("filesPage.refresh", "Refresh from server"), - section: "top", + section: "bar", order: 10, disabled: refreshing || Boolean(refreshDisabledReason), onClick: onRefresh, @@ -56,7 +56,7 @@ export function useFileLibraryWorkbenchBarButtons({ tooltip: newFolderDisabledReason ?? t("filesPage.newFolder", "New folder"), ariaLabel: t("filesPage.newFolder", "New folder"), - section: "top", + section: "bar", order: 20, disabled: Boolean(newFolderDisabledReason), onClick: onNewFolder, @@ -66,7 +66,7 @@ export function useFileLibraryWorkbenchBarButtons({ icon: , tooltip: t("filesPage.upload", "Upload"), ariaLabel: t("filesPage.upload", "Upload"), - section: "top", + section: "bar", order: 30, onClick: onUpload, }, @@ -78,7 +78,7 @@ export function useFileLibraryWorkbenchBarButtons({ icon: , tooltip: t("filesPage.uploadFromMobile", "Upload from Mobile"), ariaLabel: t("filesPage.uploadFromMobile", "Upload from Mobile"), - section: "top" as const, + section: "bar" as const, order: 40, onClick: onUploadFromMobile, }, diff --git a/frontend/editor/src/core/components/shared/WorkbenchBar.tsx b/frontend/editor/src/core/components/shared/WorkbenchBar.tsx index 5602dcd183..eed682dccb 100644 --- a/frontend/editor/src/core/components/shared/WorkbenchBar.tsx +++ b/frontend/editor/src/core/components/shared/WorkbenchBar.tsx @@ -169,6 +169,14 @@ export default function WorkbenchBar({ return selectedFileIds.length; }, [currentView, pageEditorSelectedCount, selectedFileIds.length]); + // Registered into the bar's own row rather than the tool row below it. Already + // sorted by order when registered. + const barRowButtons = useMemo( + () => + buttons.filter((btn) => btn.section === "bar" && (btn.visible ?? true)), + [buttons], + ); + const sectionsWithButtons = useMemo(() => { return SECTION_ORDER.map((section) => { const sectionButtons = buttons.filter( @@ -598,6 +606,19 @@ export default function WorkbenchBar({ {/* Right: Global buttons - export group left, close anchored right */}
+ {/* A view's own controls, ahead of the globals every view shares. */} + {barRowButtons.map((btn) => { + const content = renderButton(btn); + if (!content) return null; + return ( +
+ {content} +
+ ); + })} + {barRowButtons.length > 0 && ( +
+ )} {/* Share (viewer only; opens the same modal as My Files "Manage sharing") */} {currentView === "viewer" && sharingEnabled && ( diff --git a/frontend/editor/src/core/pages/HomePage.tsx b/frontend/editor/src/core/pages/HomePage.tsx index e0096675bd..2e4440d680 100644 --- a/frontend/editor/src/core/pages/HomePage.tsx +++ b/frontend/editor/src/core/pages/HomePage.tsx @@ -73,6 +73,15 @@ function readSwipeHintSeen(): boolean { } } +/** + * The path this last derived a view from. Module scope on purpose: HomePage remounts + * (a share link, a login bounce, a Suspense boundary resolving) and a per-mount ref + * would read the unchanged path as a fresh arrival, re-imposing the library over a + * view the user had just picked. Reset by a real page load, which is when a path does + * need deriving again. + */ +let lastSyncedPath: string | null = null; + function readPersistedSidebarCollapsed(): boolean { try { return ( @@ -231,10 +240,9 @@ export default function HomePage() { // Path moved, so the path is the cause: arrival, back/forward, or a deliberate // navigate. Mount included, which is what seeds a deep link. - const syncedPathRef = useRef(null); useEffect(() => { - if (syncedPathRef.current === location.pathname) return; - syncedPathRef.current = location.pathname; + if (lastSyncedPath === location.pathname) return; + lastSyncedPath = location.pathname; if (location.pathname.startsWith("/files")) { if (navigationState.workbench !== "myFiles") { actions.setWorkbench("myFiles"); diff --git a/frontend/editor/src/core/tests/stubbed/file-library-view.spec.ts b/frontend/editor/src/core/tests/stubbed/file-library-view.spec.ts index 96afdf00f8..e9aabd9b01 100644 --- a/frontend/editor/src/core/tests/stubbed/file-library-view.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/file-library-view.spec.ts @@ -20,6 +20,11 @@ test.describe("The file library behaves like the other views", () => { await railButton(page, /^File library$/i).click(); await expect(page).toHaveURL(/\/files/); + // The path leads the view by a render, so wait for the library itself: clicking + // the next control before it mounts races its own arrival. + await expect(page.getByRole("tree", { name: /Folders/i })).toBeVisible({ + timeout: 15_000, + }); // Reader sets the viewer workbench. The path leaving /files is that view change // reaching the URL - which is what the old reconciler undid. diff --git a/frontend/editor/src/core/types/workbenchBar.ts b/frontend/editor/src/core/types/workbenchBar.ts index 523aaab681..54e2a74b79 100644 --- a/frontend/editor/src/core/types/workbenchBar.ts +++ b/frontend/editor/src/core/types/workbenchBar.ts @@ -1,6 +1,13 @@ import React from "react"; -export type WorkbenchBarSection = "top" | "middle" | "bottom" | "tool-panel"; +/** "bar" renders in the bar's own row beside the view switcher; top/middle/bottom + * are lanes of the retractable tool row beneath it. */ +export type WorkbenchBarSection = + | "bar" + | "top" + | "middle" + | "bottom" + | "tool-panel"; export type WorkbenchBarAction = () => void;