panel names re-render when the words behind them change

Closes the gap left by the thunk: resolving at render proves nothing is cached, but
AppPicker and PanelSlot are not subscribed to anything that changes when the locale
does, so a switch updated every string in the app except the panel titles.

An opaque revision, which this package interprets in no way at all. It is a string
that changed; the caller decides what changing means. officer-web sets it to the
active locale from I18nBridge, during render rather than in an effect — an effect
leaves the first paint after a switch showing the previous language's titles.

A module-level store rather than a prop on WorkspaceView, deliberately: that
component has 28 call sites, and a prop is a thing 28 places can forget. A forgotten
one is a silently stale title, which is the failure being closed.

The new test bumps the revision and asserts the name follows with NO rerender from the
test. The existing one calls rerender, so it could only ever prove the absence of
caching — it could not have caught this.
This commit is contained in:
2026-08-23 15:55:06 +00:00
parent a36fa356ae
commit 736a45e1ff
6 changed files with 101 additions and 5 deletions
@@ -1,7 +1,8 @@
import { describe, expect, test } from 'bun:test';
import { render, screen } from '@testing-library/react';
import { I18nProvider, type TranslationMap } from 'i18n';
import { resolveEntryName } from 'workspaces/react';
import { act, render, screen } from '@testing-library/react';
import { I18nProvider, activeLocale, type TranslationMap } from 'i18n';
import { setActiveI18n } from 'i18n/standalone';
import { resolveEntryName, setNameRevision, useNameRevision } from 'workspaces/react';
import { panelName } from './panelName';
const translations: TranslationMap = {
@@ -9,8 +10,14 @@ const translations: TranslationMap = {
'pt-PT': { panel: { panelName: { fileBrowser: 'Explorador de ficheiros' } } },
};
/** Stands in for AppPicker/PanelSlot: reads the name during ITS OWN render, exactly as they do. */
const NameProbe = ({ entry }: { entry: { name: string | (() => string) } }) => <span>{resolveEntryName(entry)}</span>;
/**
* Stands in for AppPicker/PanelSlot: subscribes to the revision, then reads the name during its own
* render — exactly what those two do.
*/
const NameProbe = ({ entry }: { entry: { name: string | (() => string) } }) => {
useNameRevision();
return <span>{resolveEntryName(entry)}</span>;
};
describe('panelName', () => {
test('a plain string still works, so the 27 can migrate one at a time', () => {
@@ -41,3 +48,31 @@ describe('panelName', () => {
expect(screen.getByText('Explorador de ficheiros')).toBeDefined();
});
});
/**
* The gap the test above cannot see: it calls `rerender`, so it proves nothing is CACHED but not that
* anything asks for a new render. In the app, AppPicker and PanelSlot are not i18n consumers — a locale
* switch would update every string except the panel titles, which is the failure the revision closes.
*
* So: no rerender here. Only the revision moves, and the name has to follow on its own.
*/
describe('name revision', () => {
test('bumping the revision re-renders a name with no rerender from the test', () => {
setNameRevision('en-GB');
const entry = { name: () => (activeLocale() === 'pt-PT' ? 'Explorador' : 'File Browser') };
render(
<I18nProvider translations={translations} locale="en-GB" fallbackLocale="en-GB">
<NameProbe entry={entry} />
</I18nProvider>,
);
expect(screen.getByText('File Browser')).toBeDefined();
// What I18nBridge does on a locale change, and nothing else.
act(() => {
setActiveI18n({ translations, locale: 'pt-PT', fallbackLocale: 'en-GB' });
setNameRevision('pt-PT');
});
expect(screen.getByText('Explorador')).toBeDefined();
});
});
@@ -1,5 +1,6 @@
import type { AppRegistryMap } from './types';
import { resolveEntryName } from './types';
import { useNameRevision } from './nameRevision';
type AppPickerProps = {
registry: AppRegistryMap;
@@ -7,6 +8,8 @@ type AppPickerProps = {
};
export const AppPicker = ({ registry, onSelect }: AppPickerProps) => {
// Subscribes this component to the revision, so a name thunk is re-resolved when its words change.
useNameRevision();
const entries = Object.entries(registry).filter(([, entry]) => entry.availableOnPanel !== false);
return (
@@ -3,6 +3,7 @@ import { useCallback } from 'react';
import { ArrowLeftRight, ChevronLeft, X, Maximize2, Minimize2, ZoomIn, ZoomOut } from 'lucide-react';
import type { LayoutPanel, AppRegistryMap, PanelComponents, PanelComponentEntry } from './types';
import { resolveEntryName } from './types';
import { useNameRevision } from './nameRevision';
import { useWorkspace } from './WorkspaceContext';
import { ZOOM_MIN, ZOOM_MAX, ZOOM_STEP } from '../core/layout';
import { Card } from './internal/Card';
@@ -312,6 +313,8 @@ export const PanelSlot = ({
onSplit,
onRemove,
}: PanelSlotProps) => {
// Subscribes this component to the revision, so a name thunk is re-resolved when its words change.
useNameRevision();
const { fullscreenPanelId, transitioningPanelId, isMobile, onMobileBack } = useWorkspace();
const isFullscreen = fullscreenPanelId === panel.id;
@@ -13,6 +13,7 @@ export { WorkspaceLayout } from './WorkspaceLayout';
export { WorkspaceRenderer } from './WorkspaceRenderer';
export { PanelSlot } from './PanelSlot';
export { resolveEntryName } from './types';
export { setNameRevision, useNameRevision } from './nameRevision';
export { WorkspaceProvider, useWorkspace, inertInteraction } from './WorkspaceContext';
export type { WorkspaceContextValue } from './WorkspaceContext';
export { usePanelConfig } from './usePanelConfig';
@@ -0,0 +1,42 @@
import { useSyncExternalStore } from 'react';
/**
* An opaque revision, so a panel name re-renders when the thing that produced it changes.
*
* `AppRegistryEntry.name` may be a thunk, resolved during render rather than cached (see
* `resolveEntryName`). That is correct but not sufficient on its own: `AppPicker` and `PanelSlot` are
* not subscribed to whatever the thunk closes over, so nothing asks them to render again. A locale
* switch would update every string in the app except the panel titles.
*
* This package interprets the value in no way at all. It is a string that changed; the caller decides
* what changing means. officer-web sets it to the active locale, which is the only reason it exists —
* but the framework does not know that, and does not need a dependency on i18n to notice.
*
* A module-level store rather than a prop on `WorkspaceView`, deliberately: there are 28 call sites for
* that component, and a prop is a thing 28 places can forget. A forgotten one is a silently stale
* title, which is the exact failure this closes.
*/
let revision = '';
const listeners = new Set<() => void>();
/** Call when the words behind a name thunk may have changed. Idempotent for an unchanged value. */
export const setNameRevision = (next: string): void => {
if (next === revision) return;
revision = next;
for (const listener of listeners) listener();
};
const subscribe = (listener: () => void): (() => void) => {
listeners.add(listener);
return () => {
listeners.delete(listener);
};
};
const read = (): string => revision;
/**
* Subscribe to the revision. The return value is deliberately not useful — reading it is the point,
* because that is what makes the calling component re-render when it moves.
*/
export const useNameRevision = (): string => useSyncExternalStore(subscribe, read, read);