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.
This commit is contained in:
Reece
2026-09-01 14:34:04 +01:00
parent 56a90b543a
commit 16b297f488
5 changed files with 49 additions and 8 deletions
@@ -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: <UploadFileIcon />,
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: <QrCode2Icon />,
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,
},
@@ -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 */}
<div className="workbench-bar-globals">
{/* A view's own controls, ahead of the globals every view shares. */}
{barRowButtons.map((btn) => {
const content = renderButton(btn);
if (!content) return null;
return (
<div key={btn.id} className="workbench-bar-action-wrapper">
{content}
</div>
);
})}
{barRowButtons.length > 0 && (
<div className="workbench-bar-divider workbench-bar-globals-sep" />
)}
{/* Share (viewer only; opens the same modal as My Files "Manage sharing") */}
{currentView === "viewer" && sharingEnabled && (
<ViewerShareButton disabled={actionsDisabled} />
+11 -3
View File
@@ -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<string | null>(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");
@@ -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.
@@ -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;