diff --git a/.changeset/record-header-manual-refresh-3460.md b/.changeset/record-header-manual-refresh-3460.md new file mode 100644 index 0000000000..fbca14bf6b --- /dev/null +++ b/.changeset/record-header-manual-refresh-3460.md @@ -0,0 +1,17 @@ +--- +"@object-ui/app-shell": patch +"@object-ui/components": patch +"@object-ui/plugin-detail": patch +--- + +Record detail pages: a header ⟳ that refreshes the record, its related lists and its tab counts in place — no browser reload + +Concurrent-editing scenario from the shop floor (MES work orders): operator A sits on a record's detail page while operator B starts or reports the same order. A had no way to see the new state except F5, which throws away the open tab, the scroll position and any in-progress inline edit along with the stale data. + +The pipeline for this already existed — the objectui#2269 invalidation bus refetches every mounted reader in place, and `RecordContext.refresh` had been declared for it — but nothing produced that field and no UI reached for it. Three changes give it a trigger: + +- **`RecordDetailView` produces `RecordContext.refresh`**, publishing `notifyDataChanged({ objectName: '*' })`. The wildcard is deliberate: a user reaches for refresh because of a write made by SOMEONE ELSE, which this client never saw and therefore cannot attribute to particular objects. `'*'` marks everything mounted as stale, so the main record, every related child list and the tab-count badges all refetch — no remount, so tab / scroll / draft state survive. First phase covers the standalone record route; embedded hosts (list drawer, split-pane preview) keep their existing chrome unchanged. +- **`page:header` renders the ⟳** at the far end of the header row when — and only when — the host provides `refresh`. It is page chrome rather than a header action, so its position is the same on every record page regardless of which business actions the object declares, and it can never be collapsed into the `⋯` overflow. Styled as that `⋯` trigger's twin so the row reads as one button family. Its accessible name and tooltip come from the existing `common.refresh` key, so the icon-only button is not English-only in the other nine locales. The icon spins for a short floor after a click, because the bus is fire-and-forget and a warm backend would otherwise finish before the click looked like it landed. +- **`RelatedList` accepts the `'*'` wildcard** on the legacy `objectui:related-changed` event, matching what `dataChangeMatches` already does for the bus's own readers. This listener compared the payload's object name to its own, so a wildcard invalidation reached everything on the page except the related lists — a concrete foreign object name is still ignored. + +Hosts that provide no `refresh` render exactly as before. diff --git a/content/docs/guide/slotted-pages.md b/content/docs/guide/slotted-pages.md index 17f16736ef..3b1cc8a544 100644 --- a/content/docs/guide/slotted-pages.md +++ b/content/docs/guide/slotted-pages.md @@ -159,6 +159,32 @@ slots: { }, ``` +### The refresh button is chrome, not an action + +Past the `⋯` menu, at the far end of the row, a record page shows a **⟳ +refresh** button. It is deliberately *not* part of the action row: + +- **Nothing to author.** It appears when the page **host** supplies + `RecordContext.refresh` — the standalone record route does, so every + record page has it, in the same place, whatever actions the object + declares. There is no metadata key for it and no `locations` to write. +- **Never collapsed.** Because it is outside the action list, it is not + part of the `maxVisible` budget and can never be pushed into the `⋯` + menu by an object that declares many actions. +- **Not permission-gated.** Re-reading a record already on screen is not + a privileged operation, so it skips the `requiredPermissions` gate the + action pipeline applies. + +Clicking it invalidates data on the client bus (`notifyDataChanged` from +`@object-ui/react`) with the wildcard scope `'*'`: the record, every +related list and the tab-count badges refetch **in place**. Nothing +remounts, so the open tab, the scroll position and any in-progress +inline edit survive the refresh — the point of the button is to see +another user's writes without an F5. + +A host that supplies no `refresh` (an embedded drawer, a designer +preview, a non-record page) renders no button. + ## Composing default + custom When you want "the default actions plus one custom button," you have diff --git a/packages/app-shell/src/views/RecordDetailView.headerRefresh.test.tsx b/packages/app-shell/src/views/RecordDetailView.headerRefresh.test.tsx new file mode 100644 index 0000000000..ddb9c89749 --- /dev/null +++ b/packages/app-shell/src/views/RecordDetailView.headerRefresh.test.tsx @@ -0,0 +1,232 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * RecordDetailView — the PRODUCER for the header's manual ⟳ (objectui#3460). + * + * `RecordContextValue.refresh` has been declared (and consumed nowhere) since + * the context was introduced: a field the types, the memo dep list and lint all + * accepted while no code path ever wrote it. This view is its producer, and + * `page:header` (see `page-header-refresh.test.tsx` in @object-ui/components) + * is the consumer. + * + * Two properties of the wiring can only be checked here, on the host: + * + * 1. The published scope is the `'*'` WILDCARD, not this record. The reason a + * user reaches for refresh is a write made by someone else — a write this + * client never saw, so it cannot know which objects it touched. `'*'` is + * the bus's "everything is stale", which is what also reaches the related + * lists and the tab-count badges. A record-scoped notify would look + * identical on the main record and silently leave those stale, so the + * payload is asserted on the bus itself, not inferred from a refetch. + * 2. The record refetches IN PLACE (AGENTS.md #8 / objectui#2269): the same + * `findOne` runs again and the header DOM node survives. A `key=` bump + * would also produce a second `findOne` while destroying tab, scroll and + * inline-edit state, so "it refetched" alone is not the property. + * + * The embedded case pins the #3460 ruling's scope decision: the list drawer and + * the split-pane preview mount THIS view with `embedded` (ObjectView, + * ObjectDataPage, InterfaceListPage all do), so the first phase's "no ⟳ in + * drawer/split-pane" has to be enforced right here at the producer. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, waitFor, cleanup, fireEvent } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { MetadataCtx, subscribeDataChanges } from '@object-ui/react'; +import type { DataChange } from '@object-ui/react'; + +vi.mock('@object-ui/auth', () => ({ + useAuth: () => ({ user: { id: 'u1', name: 'Ada', image: null }, activeOrganization: null }), + createAuthenticatedFetch: () => vi.fn(), +})); + +vi.mock('@object-ui/collaboration', () => ({ + useRecordPresence: () => ({ viewers: [], others: [] }), + PresenceAvatars: () => null, +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), + error: vi.fn(), + info: vi.fn(), + warning: vi.fn(), + loading: vi.fn(), + dismiss: vi.fn(), + }), +})); + +// Orthogonal chrome — same posture as RecordDetailView.feedLoading.test. +vi.mock('./ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null })); +vi.mock('./ActionParamDialog', () => ({ ActionParamDialog: () => null })); +vi.mock('./ActionResultDialog', () => ({ ActionResultDialog: () => null })); +vi.mock('./FlowRunner', () => ({ FlowRunner: () => null })); +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); + +import { RecordDetailView } from './RecordDetailView'; + +const OBJECT_NAME = 'mes_work_order'; +const RECORD_ID = 'WO-1'; + +const OBJECTS = [ + { + name: OBJECT_NAME, + label: 'Work Order', + managedBy: 'platform', + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + status: { type: 'text', label: 'Status' }, + }, + }, +]; + +function makeDataSource() { + // Each read answers a fresh status so a refetch is observable in the DOM as + // well as in the call count. + let generation = 0; + const findOne = vi.fn(async () => { + generation += 1; + return { id: RECORD_ID, name: 'WO-0001', status: `gen-${generation}` }; + }); + return { + find: vi.fn(async () => ({ data: [] })), + findOne, + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + } as any; +} + +function makeMetadata() { + return { + objects: OBJECTS, + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], + } as any; +} + +function renderDetail(dataSource: any, opts?: { embedded?: boolean }) { + return render( + + + {}} + objectNameOverride={OBJECT_NAME} + recordIdOverride={RECORD_ID} + embedded={opts?.embedded} + /> + + , + ); +} + +const refreshButton = () => document.querySelector('[data-page-refresh]') as HTMLButtonElement | null; + +beforeEach(() => { + cleanup(); + // Unrelated chrome (approvals, favourites) reaches for the platform API; in + // jsdom that is a real socket. Answer locally so the only asynchrony here is + // the record read. + vi.stubGlobal( + 'fetch', + vi.fn(async () => + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); +}); + +describe('RecordDetailView — the header ⟳ refetches in place (objectui#3460)', () => { + it('renders the ⟳ on the standalone record route', async () => { + const dataSource = makeDataSource(); + renderDetail(dataSource); + + await waitFor(() => expect(refreshButton()).toBeTruthy()); + }); + + it('publishes the WILDCARD scope on the invalidation bus, not this record', async () => { + const dataSource = makeDataSource(); + const changes: DataChange[] = []; + const unsubscribe = subscribeDataChanges((c) => changes.push(c)); + try { + renderDetail(dataSource); + await waitFor(() => expect(refreshButton()).toBeTruthy()); + + fireEvent.click(refreshButton()!); + + expect(changes).toEqual([{ objectName: '*' }]); + // Explicitly NOT the record scope: that would refresh the main record and + // leave every related list and count badge stale. + expect(changes[0]).not.toHaveProperty('recordId', RECORD_ID); + } finally { + unsubscribe(); + } + }); + + it('re-runs the record read without remounting the page', async () => { + const dataSource = makeDataSource(); + renderDetail(dataSource); + + await waitFor(() => expect(refreshButton()).toBeTruthy()); + await waitFor(() => expect(dataSource.findOne).toHaveBeenCalledTimes(1)); + const headerBefore = document.querySelector('header'); + expect(headerBefore).toBeTruthy(); + + fireEvent.click(refreshButton()!); + + // The bus re-runs the SAME load effect… + await waitFor(() => expect(dataSource.findOne).toHaveBeenCalledTimes(2)); + // …and the header is the same DOM node it was before the click. A `key=` + // bump would have refetched too, while throwing away tab/scroll/inline-edit + // state (AGENTS.md #8). + expect(document.querySelector('header')).toBe(headerBefore); + expect(refreshButton()).toBeTruthy(); + }); +}); + +describe('RecordDetailView — embedded hosts opt out of the ⟳ (objectui#3460 ruling)', () => { + it('renders no ⟳ in a drawer / split-pane host', async () => { + const dataSource = makeDataSource(); + renderDetail(dataSource, { embedded: true }); + + // Wait for the record read to settle so this is "the header rendered and + // has no ⟳", not "the header hasn't rendered yet". + await waitFor(() => expect(dataSource.findOne).toHaveBeenCalled()); + await waitFor(() => expect(document.querySelector('header')).toBeTruthy()); + expect(refreshButton()).toBeNull(); + }); + + it('publishes nothing on the bus for an embedded host', async () => { + const dataSource = makeDataSource(); + const changes: DataChange[] = []; + const unsubscribe = subscribeDataChanges((c) => changes.push(c)); + try { + renderDetail(dataSource, { embedded: true }); + await waitFor(() => expect(dataSource.findOne).toHaveBeenCalled()); + expect(changes).toEqual([]); + } finally { + unsubscribe(); + } + }); +}); diff --git a/packages/app-shell/src/views/RecordDetailView.tsx b/packages/app-shell/src/views/RecordDetailView.tsx index 908ee274c7..0be091d303 100644 --- a/packages/app-shell/src/views/RecordDetailView.tsx +++ b/packages/app-shell/src/views/RecordDetailView.tsx @@ -364,6 +364,26 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri const notifyRecordChanged = useCallback(() => { if (objectName) notifyDataChanged({ objectName, recordId: pureRecordId || undefined }); }, [objectName, pureRecordId]); + // Manual refresh (objectui#3460) — the producer for `RecordContext.refresh`, + // which `page:header` turns into the ⟳ button at the end of the header row. + // + // Scope is deliberately `'*'`, not this record: the reason a user reaches for + // refresh is a write made by SOMEONE ELSE (another operator started the work + // order, a child row got reported) — a write this client never saw, so it + // cannot know which objects it touched. `'*'` treats everything mounted as + // stale, so the main record, every related child list and the tab-count + // badges all refetch in place over the #2269 bus: no remount, so tab / + // scroll / in-progress inline-edit state all survive. + const handleManualRefresh = useCallback(() => { + notifyDataChanged({ objectName: '*' }); + }, []); + // First phase covers the standalone record ROUTE only. Embedded hosts — the + // list drawer and the split-pane preview (ObjectView / ObjectDataPage / + // InterfaceListPage all mount this same view with `embedded`) — wrap it in + // overlay chrome that already owns its own controls, and the #3460 ruling + // keeps ⟳ off those surfaces for now. `undefined` is the opt-out + // `page:header` reads, so nothing renders there. + const headerRefresh = embedded ? undefined : handleManualRefresh; // Record-scoped presence ("who else is viewing this record"). The default // PresenceProvider source is a no-op, so this resolves to `[]` until a @@ -2110,6 +2130,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri objectSchema={objectDef} dataSource={dataSource} embedded={embedded} + refresh={headerRefresh} headerSystemActions={synthSystemActions} isFavorite={isRecordFavorite} onToggleFavorite={favoriteRecord ? handleToggleRecordFavorite : undefined} diff --git a/packages/components/src/__tests__/page-header-refresh.test.tsx b/packages/components/src/__tests__/page-header-refresh.test.tsx new file mode 100644 index 0000000000..924fc416b9 --- /dev/null +++ b/packages/components/src/__tests__/page-header-refresh.test.tsx @@ -0,0 +1,253 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `page:header` — the manual refresh ⟳ (objectui#3460). + * + * MES shop-floor scenario: operator A sits on a work-order detail page while + * operator B starts/reports the same order. A had no way to see it but F5. + * The header now offers a ⟳ that publishes on the #2269 invalidation bus, so + * the record, its related lists and the tab-count badges refetch IN PLACE. + * + * What this file pins, and why each one is a separate failure mode: + * + * 1. The button is HOST-OPTED-IN — it renders iff `RecordContext.refresh` + * exists. Every other host (previews, the embedded drawer, non-record + * pages) must render byte-for-byte what it rendered before, so the + * no-refresh cases assert the *absence* of the button AND the survival of + * the `data-page-actions-slot` layout div. + * 2. It is page CHROME, not a header action: it lives OUTSIDE the actions + * `role="toolbar"`, at the far end of the row, and therefore cannot be + * collapsed into the `⋯` overflow when an object declares more actions + * than `maxVisible`. That property is the whole reason it isn't wired + * through the action pipeline, so it is asserted rather than assumed. + * 3. Its accessible name comes from `common.refresh` — an existing key in all + * ten packs — so an icon-only button is not English-only for the nine + * other locales (the same failure objectstack#5407 fixed for `⋯`). + * 4. Clicking calls the host's refresh exactly once and spins for a floor of + * ~650ms. The bus is fire-and-forget, so without the floor a warm backend + * renders the click as "nothing happened". + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { act, render, screen, cleanup, fireEvent } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { ActionProvider, RecordContextProvider } from '@object-ui/react'; +import { I18nProvider } from '@object-ui/i18n'; +// Registers `page:header` at module scope, NOT inside a `beforeAll` — there the +// cold transform is billed to `hookTimeout` +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010). +import '../renderers'; + +/** Must track MANUAL_REFRESH_SPIN_MS in `renderers/layout/containers.tsx`. */ +const SPIN_MS = 650; + +function PageHeader({ schema }: { schema: any }) { + const Component = ComponentRegistry.get('page:header'); + if (!Component) throw new Error('page:header not registered'); + // eslint-disable-next-line react-hooks/static-components -- ComponentRegistry.get returns a registered component (stable), not one created during render + return ; +} + +const OBJECT_SCHEMA = { name: 'mes_work_order', label: 'Work Order' }; +const RECORD = { id: 'WO-1', name: 'WO-0001' }; + +interface Opts { + refresh?: () => void; + /** Omit for "the record hasn't loaded yet" (the bare-title header branch). */ + record?: any; + actions?: any[]; + language?: string; +} + +function renderHeader(opts: Opts = {}) { + const schema: any = { type: 'page:header', title: 'Work Order' }; + if (opts.actions) schema.actions = opts.actions; + return render( + + + + + + + , + ); +} + +const refreshButton = () => document.querySelector('[data-page-refresh]') as HTMLButtonElement | null; +const spinIcon = () => refreshButton()?.querySelector('svg')?.getAttribute('class') ?? ''; + +afterEach(() => { + cleanup(); + vi.useRealTimers(); +}); + +describe('page:header — the ⟳ is host-opted-in (objectui#3460)', () => { + it('renders no ⟳ when the host provides no refresh', () => { + renderHeader(); + + expect(refreshButton()).toBeNull(); + expect(screen.queryByRole('button', { name: 'Refresh' })).toBeNull(); + }); + + it('leaves a no-refresh header exactly as it was — actions slot included', () => { + const { container } = renderHeader(); + + // The empty-actions layout placeholder must still be the header's own + // trailing child, not moved inside a new wrapper: this is the regression + // guard for every host that never opts in. + const slot = container.querySelector('[data-page-actions-slot]'); + expect(slot).toBeTruthy(); + expect(slot!.parentElement?.tagName).toBe('HEADER'); + }); + + it('renders the ⟳ once the host provides refresh', () => { + renderHeader({ refresh: vi.fn() }); + + expect(refreshButton()).toBeTruthy(); + expect(screen.getByRole('button', { name: 'Refresh' })).toBeTruthy(); + }); + + it('renders the ⟳ before the record has loaded (bare-title header branch)', () => { + // A record page's first paint has no `ctx.data`, so `page:header` takes its + // non-record layout. The button must not pop in and out between branches. + renderHeader({ refresh: vi.fn(), record: undefined }); + + expect(refreshButton()).toBeTruthy(); + }); +}); + +describe('page:header — the ⟳ is chrome, not an action (objectui#3460)', () => { + it('sits at the far end of the header row, after the ⋯ overflow trigger', () => { + // Four actions against the default maxVisible of 3 → three inline buttons + // plus the ⋯ trigger. The ⟳ comes last of all. + renderHeader({ + refresh: vi.fn(), + actions: [ + { name: 'start', locations: ['record_header'], label: 'Start' }, + { name: 'expedite', locations: ['record_header'], label: 'Expedite' }, + { name: 'edit', locations: ['record_header'], label: 'Edit' }, + { name: 'archive', locations: ['record_header'], label: 'Archive' }, + ], + }); + + const buttons = Array.from(document.querySelectorAll('header button')); + expect(buttons.length).toBeGreaterThan(4); + expect(buttons[buttons.length - 1]).toBe(refreshButton()); + // …and the one before it is the overflow trigger, i.e. the ⟳ did not steal + // an inline slot from the actions. + expect(buttons[buttons.length - 2]?.getAttribute('aria-label')).toBe('More actions'); + }); + + it('stays out of the actions toolbar so the overflow budget cannot swallow it', () => { + renderHeader({ + refresh: vi.fn(), + actions: [ + { name: 'a', locations: ['record_header'], label: 'A' }, + { name: 'b', locations: ['record_header'], label: 'B' }, + { name: 'c', locations: ['record_header'], label: 'C' }, + { name: 'd', locations: ['record_header'], label: 'D' }, + ], + }); + + const toolbar = document.querySelector('[role="toolbar"]'); + expect(toolbar).toBeTruthy(); + expect(toolbar!.contains(refreshButton())).toBe(false); + // Not routed into the ⋯ menu either — it is still a visible button. + expect(refreshButton()).toBeTruthy(); + }); + + it('renders alongside the actions when the host also injects system actions', () => { + renderHeader({ + refresh: vi.fn(), + actions: [{ name: 'start', locations: ['record_header'], label: 'Start' }], + }); + + expect(screen.getByRole('button', { name: 'Start' })).toBeTruthy(); + expect(refreshButton()).toBeTruthy(); + }); +}); + +describe('page:header — the ⟳ speaks the session locale (objectui#3460)', () => { + it('reads common.refresh under a zh session', () => { + renderHeader({ refresh: vi.fn(), language: 'zh' }); + + expect(screen.getByRole('button', { name: '刷新' })).toBeTruthy(); + // The English literal asserted negatively too — a re-inlined default would + // otherwise pass the positive assertion on a header with two buttons. + expect(screen.queryByRole('button', { name: 'Refresh' })).toBeNull(); + }); + + it('reads common.refresh under a ja session', () => { + renderHeader({ refresh: vi.fn(), language: 'ja' }); + + expect(screen.getByRole('button', { name: '更新' })).toBeTruthy(); + }); + + it('still reads English under an en session', () => { + renderHeader({ refresh: vi.fn(), language: 'en' }); + + expect(screen.getByRole('button', { name: 'Refresh' })).toBeTruthy(); + }); +}); + +describe('page:header — clicking the ⟳ (objectui#3460)', () => { + it("calls the host's refresh exactly once per click", () => { + const refresh = vi.fn(); + renderHeader({ refresh }); + + fireEvent.click(refreshButton()!); + expect(refresh).toHaveBeenCalledTimes(1); + + fireEvent.click(refreshButton()!); + expect(refresh).toHaveBeenCalledTimes(2); + }); + + it('spins for the whole feedback floor, then stops', () => { + const refresh = vi.fn(); + renderHeader({ refresh }); + expect(spinIcon()).not.toContain('animate-spin'); + + vi.useFakeTimers(); + fireEvent.click(refreshButton()!); + expect(spinIcon()).toContain('animate-spin'); + + // Still spinning just short of the floor… + act(() => { vi.advanceTimersByTime(SPIN_MS - 50); }); + expect(spinIcon()).toContain('animate-spin'); + + // …and stopped once it elapses. + act(() => { vi.advanceTimersByTime(60); }); + expect(spinIcon()).not.toContain('animate-spin'); + }); + + it('restarts the floor on a second click instead of ending early', () => { + const refresh = vi.fn(); + renderHeader({ refresh }); + + vi.useFakeTimers(); + fireEvent.click(refreshButton()!); + act(() => { vi.advanceTimersByTime(SPIN_MS - 100); }); + fireEvent.click(refreshButton()!); + // The first click's timer would have fired here; the second click's floor + // must still be running. + act(() => { vi.advanceTimersByTime(150); }); + expect(spinIcon()).toContain('animate-spin'); + + act(() => { vi.advanceTimersByTime(SPIN_MS); }); + expect(spinIcon()).not.toContain('animate-spin'); + }); +}); diff --git a/packages/components/src/renderers/layout/containers.tsx b/packages/components/src/renderers/layout/containers.tsx index ee54129a30..54bac35bc5 100644 --- a/packages/components/src/renderers/layout/containers.tsx +++ b/packages/components/src/renderers/layout/containers.tsx @@ -45,10 +45,26 @@ import { DropdownMenuTrigger, DropdownMenuContent, DropdownMenuItem, + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, } from '../../ui'; import { RecordTitleChip } from '../../custom/RecordTitleChip'; import { useObjectLabel, useSafeFieldLabel, useObjectTranslation, useSafeTranslate, createSafeTranslation, pickLocalized } from '@object-ui/i18n'; -import { MoreHorizontal } from 'lucide-react'; +import { MoreHorizontal, RefreshCw } from 'lucide-react'; + +/** + * How long the header's manual-refresh icon keeps spinning after a click + * (objectui#3460). + * + * The refresh itself is fire-and-forget — it publishes on the #2269 + * invalidation bus and every mounted reader refetches in place, so there is no + * completion signal to spin until. Against a warm backend the whole round trip + * can finish inside one frame, which would render the click as "nothing + * happened". This floor makes the click visibly land. + */ +const MANUAL_REFRESH_SPIN_MS = 650; /** * Copy for the `page:tabs` count badge (objectstack#5506). @@ -914,6 +930,31 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { // the SAME key `action:menu`'s overflow trigger already reads, so the two // `⋯` buttons a record page can show cannot read differently per locale. const tt = useSafeTranslate(); + // ── Manual refresh (objectui#3460) ──────────────────────────────────────── + // Rendered as page CHROME at the far end of the header row, NOT as a header + // action: business/system actions come and go per object and record state, + // while refresh has to sit in the same place on every record page. Keeping it + // out of the action pipeline also keeps it out of that pipeline's capability + // gating and `maxVisible` overflow budget — reading a record you are already + // looking at is not a permissioned operation, and the button must never be + // the one that gets collapsed into `⋯`. + // + // The host opts in by providing `RecordContext.refresh`; hosts that don't + // (previews, embedded drawers, non-record pages) render exactly as before. + const hostRefresh = ctx?.refresh; + const [manualRefreshing, setManualRefreshing] = React.useState(false); + const refreshSpinTimer = React.useRef | undefined>(undefined); + React.useEffect(() => () => clearTimeout(refreshSpinTimer.current), []); + const handleManualRefresh = React.useCallback(() => { + if (!hostRefresh) return; + void hostRefresh(); + setManualRefreshing(true); + clearTimeout(refreshSpinTimer.current); + refreshSpinTimer.current = setTimeout( + () => setManualRefreshing(false), + MANUAL_REFRESH_SPIN_MS, + ); + }, [hostRefresh]); // Spec bridge may either inline `properties.*` onto the node or preserve // the raw bag (see record:quick_actions for the same pattern). Read from // both so a `{ properties: { title } }` schema is rendered correctly. @@ -1327,6 +1368,60 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { ); }; + /** + * The ⟳ button (objectui#3460), or `null` when the host provides no + * `refresh`. + * + * Styled as the `⋯` overflow trigger's twin — same `outline` variant, same + * `size="sm"`, same padding — so the whole row reads as ONE button family + * (`[Start][Expedite][Edit][⋯][⟳]`). A bare ghost icon next to a row of + * bordered pills read as detached from it. + */ + const renderRefreshButton = (): React.ReactNode => { + if (!hostRefresh) return null; + const refreshLabel = tt('common.refresh', 'Refresh'); + return ( + + + + + + {refreshLabel} + + + ); + }; + + /** + * The header's trailing slot: the action row (or the empty layout slot that + * stands in for it) followed by the refresh chrome. + * + * Both header layouts below funnel through this so the ⟳ keeps the same + * position whether or not the record chip is rendered — including the first + * paint of a record page, before `ctx.data` lands. When no host provides + * `refresh` the slot is returned untouched, byte for byte what it was. + */ + const renderHeaderTail = (emptySlot: React.ReactNode): React.ReactNode => { + const actions = renderHeaderActions() ?? emptySlot; + const refreshButton = renderRefreshButton(); + if (!refreshButton) return actions; + return ( +
+ {actions} + {refreshButton} +
+ ); + }; + // Decide whether to render the record chip. Conditions: // 1. There's a live RecordContext with data + an object schema. // 2. Author hasn't opted out via `recordChrome: false`. @@ -1420,7 +1515,7 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => {

{subtitle}

)} - {renderHeaderActions() ??
} + {renderHeaderTail(
)} ); } @@ -1442,7 +1537,7 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { )} {subtitle &&

{subtitle}

}
- {renderHeaderActions() ??
} + {renderHeaderTail(
)}
); diff --git a/packages/plugin-detail/src/RelatedList.tsx b/packages/plugin-detail/src/RelatedList.tsx index f41fe2b17b..1cb2e96100 100644 --- a/packages/plugin-detail/src/RelatedList.tsx +++ b/packages/plugin-detail/src/RelatedList.tsx @@ -509,11 +509,18 @@ export const RelatedList: React.FC = ({ // e.g. a child row action executed through the host retargets `api` and // dispatches `objectui:related-changed`. Only meaningful on the auto-fetch // path (parent-provided data is refreshed by the parent). + // + // `'*'` is the invalidation bus's "unknown scope — everything is stale" + // wildcard (undo of an unknown operation, the record header's manual ⟳ in + // objectui#3460): it must match EVERY list, exactly as `dataChangeMatches` in + // `@object-ui/react` already does for the bus's own readers. A concrete + // object name still has to be this list's own — a write to some other object + // is not a reason to refetch here. React.useEffect(() => { if (!api || dataProvided) return; const onChanged = (ev: Event) => { const detail = (ev as CustomEvent).detail || {}; - if (detail.objectName && detail.objectName !== api) return; + if (detail.objectName && detail.objectName !== '*' && detail.objectName !== api) return; setRefreshNonce((n) => n + 1); }; window.addEventListener('objectui:related-changed', onChanged as EventListener); diff --git a/packages/plugin-detail/src/__tests__/RelatedList.wildcardInvalidation.test.tsx b/packages/plugin-detail/src/__tests__/RelatedList.wildcardInvalidation.test.tsx new file mode 100644 index 0000000000..d20e639aab --- /dev/null +++ b/packages/plugin-detail/src/__tests__/RelatedList.wildcardInvalidation.test.tsx @@ -0,0 +1,135 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * RelatedList — the `'*'` wildcard on the legacy invalidation event + * (objectui#3460). + * + * `notifyDataChanged` (the #2269 bus) still dispatches the pre-bus + * `objectui:related-changed` window event for backward compatibility, and this + * list is that event's remaining listener. The bus's own readers go through + * `dataChangeMatches`, which treats `objectName: '*'` as "unknown scope — + * everything is stale"; this listener predates that and compared the payload's + * object name to its own, so a wildcard invalidation reached every bus reader + * on the page EXCEPT the related lists. + * + * That is what made the record header's manual ⟳ (#3460) a half-refresh: it + * publishes `'*'` precisely because a click means "someone else wrote data I + * can't identify", the main record and the tab-count badges refetched, and the + * child rows underneath them did not. + * + * Both directions are pinned, because the fix is a widening and the guard it + * widens still has a job: `'*'` must be accepted, and a CONCRETE foreign object + * name must still be ignored. Dropping the second assertion would let + * "refetch on every event" pass as a fix. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, waitFor, act } from '@testing-library/react'; +import * as React from 'react'; +import { RelatedList } from '../RelatedList'; + +// Capture the schema RelatedList hands to SchemaRenderer (the data-table) so +// the rows a refetch produced are observable without rendering the grid. +const h = vi.hoisted(() => ({ schema: null as any })); +vi.mock('@object-ui/react', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + SchemaRenderer: (props: any) => { + h.schema = props.schema; + return null; + }, + }; +}); + +const RELATED_OBJECT = 'mes_work_order_line'; +const columns = [{ accessorKey: 'name', header: 'Name' }]; + +/** DataSource stub whose rows change per read, so a refetch is observable. */ +function makeDataSource() { + let generation = 0; + const find = vi.fn(async () => { + generation += 1; + return { data: [{ id: `line-${generation}`, name: `Line ${generation}` }] }; + }); + return { find }; +} + +function renderRelated(dataSource: any) { + return render( + , + ); +} + +/** Dispatch the legacy event the bus emits for backward compatibility. */ +function dispatchRelatedChanged(detail: Record) { + act(() => { + window.dispatchEvent(new CustomEvent('objectui:related-changed', { detail })); + }); +} + +beforeEach(() => { + h.schema = null; +}); + +describe("RelatedList — legacy related-changed listener and '*' (objectui#3460)", () => { + it("refetches on the '*' wildcard scope", async () => { + const ds = makeDataSource(); + renderRelated(ds); + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(1)); + + dispatchRelatedChanged({ objectName: '*' }); + + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(2)); + // The refetched rows actually reached the table — not just a call count. + await waitFor(() => expect(h.schema?.data?.[0]?.id).toBe('line-2')); + }); + + it('still refetches on its own object name', async () => { + const ds = makeDataSource(); + renderRelated(ds); + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(1)); + + dispatchRelatedChanged({ objectName: RELATED_OBJECT }); + + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(2)); + }); + + it('still refetches on a scopeless event (no objectName in the detail)', async () => { + const ds = makeDataSource(); + renderRelated(ds); + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(1)); + + dispatchRelatedChanged({}); + + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(2)); + }); + + it('still IGNORES a concrete foreign object name', async () => { + // The other half of the guard. A write to some unrelated object is not a + // reason for this list to spend a round trip — widening `'*'` must not + // widen this. + const ds = makeDataSource(); + renderRelated(ds); + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(1)); + + dispatchRelatedChanged({ objectName: 'crm_contact' }); + dispatchRelatedChanged({ objectName: 'mes_work_order' }); + dispatchRelatedChanged({ objectName: `${RELATED_OBJECT}_history` }); + + // Give any (wrong) refetch several turns to show up before asserting none. + await new Promise((r) => setTimeout(r, 20)); + expect(ds.find).toHaveBeenCalledTimes(1); + expect(h.schema?.data?.[0]?.id).toBe('line-1'); + }); +});