fix(ui): keep the header actions in fixed slots instead of a pager prop

Reverts the `pager` slot on AppHeaderActions. The block is right-pinned, so
ordering alone gives the guarantee the slot was reaching for: put the controls
that come and go at the start of the children and everything after them keeps
its position. Flow header, right to left: actions menu, pager, favourite,
report — the report being the one that waits on the task list.

The controls that are always meaningful for a route are now always rendered
and disabled from an explicit loading flag rather than unmounted when the
entity object is falsy. Stepping used to collapse the whole cluster to a lone
star for the length of the fetch, and the pager — which needs the sibling list,
not the current entity — went with it, so a second step meant waiting.
Templates and knowledge get the same treatment; knowledge had no loading
signal at all, so one is threaded down from the page.

Two side effects of dropping the entity gates: on phones the flows row and the
favourite toggle survive an unloaded list (they were nested behind it), and the
position counter reserves the widest label its total can produce, so stepping
across a digit boundary no longer slides Previous out from under the cursor.

The pixel baselines cannot pin any of this — the cluster is far below the
visual project's diff ratio — so the order is asserted in the spec instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Sergey Kozyrenko
2026-07-22 16:05:36 +07:00
co-authored by Claude Opus 4.8
parent 5b4ded1e3c
commit ee6ed49d48
14 changed files with 186 additions and 179 deletions
@@ -24,8 +24,17 @@ test.describe('responsive', { tag: '@cross' }, () => {
expect(await page.evaluate(hasHorizontalOverflow)).toBe(false);
await page.getByRole('row', { name: /E2E Alpha/ }).click();
// The title, not the actions trigger: below md the star and the pager live inside
// the menu, and the trigger itself renders before the flow arrives.
await expect(page.locator('header').getByText('E2E Alpha')).toBeVisible();
await expect(page.getByRole('button', { name: 'Flow actions' })).toBeVisible();
expect(await page.evaluate(hasHorizontalOverflow)).toBe(false);
await page.getByRole('button', { name: 'Flow actions' }).click();
await expect(page.getByRole('menuitem', { name: /Flows/ })).toBeVisible();
await expect(page.getByRole('menuitem', { name: /favorites/ })).toBeVisible();
await page.keyboard.press('Escape');
expectCleanPage(pageErrorLog);
});
});
+1 -1
View File
@@ -43,7 +43,7 @@ test.describe('flow create', { tag: ['@flows', '@smoke'] }, () => {
await page.getByRole('button', { name: 'Submit' }).click();
await expect(page).toHaveURL(/\/flows\/7/);
await expect(page.getByRole('button', { name: 'Flow actions' })).toBeVisible();
await expect(page.locator('header').getByRole('button', { name: 'Toggle favorite' })).toBeEnabled();
await expect(page.locator('header').getByText('Say Hello Flow')).toBeVisible();
expectCleanPage(pageErrorLog);
});
+1 -1
View File
@@ -17,7 +17,7 @@ import { FLOW_A, flowsCassette, makeFlow } from '../../mocks/cassettes/flows.ts'
const openFlowA = async (page: import('@playwright/test').Page) => {
await page.goto('/flows');
await page.getByRole('row', { name: /E2E Alpha/ }).click();
await expect(page.getByRole('button', { name: 'Flow actions' })).toBeVisible();
await expect(page.locator('header').getByRole('button', { name: 'Toggle favorite' })).toBeEnabled();
};
test.describe('flow lifecycle', { tag: '@flows' }, () => {
+2 -2
View File
@@ -21,7 +21,7 @@ test.describe('flow detail tabs', { tag: '@flows' }, () => {
test('each tab renders its populated content', async ({ page, pageErrorLog }) => {
await page.goto('/flows/5');
await expect(page.getByRole('button', { name: 'Flow actions' })).toBeVisible();
await expect(page.locator('header').getByRole('button', { name: 'Toggle favorite' })).toBeEnabled();
for (const { marker, name } of TABS) {
await page.getByRole('tab', { name }).click();
@@ -33,7 +33,7 @@ test.describe('flow detail tabs', { tag: '@flows' }, () => {
test('the screenshot image decodes from its REST endpoint', async ({ page, pageErrorLog }) => {
await page.goto('/flows/5');
await expect(page.getByRole('button', { name: 'Flow actions' })).toBeVisible();
await expect(page.locator('header').getByRole('button', { name: 'Toggle favorite' })).toBeEnabled();
await page.getByRole('tab', { name: 'Screenshots' }).click();
const image = page.getByRole('img', { name: TABS_SCREENSHOT_NAME });
Binary file not shown.

Before

Width:  |  Height:  |  Size: 58 KiB

After

Width:  |  Height:  |  Size: 58 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 56 KiB

After

Width:  |  Height:  |  Size: 56 KiB

@@ -55,23 +55,10 @@ export function AppHeaderAction({
);
}
export function AppHeaderActions({
children,
className,
pager,
}: {
children?: ReactNode;
className?: string;
pager?: ReactNode;
}) {
// Pass a detail prev/next control as `pager`, not as a child: as the trailing child it stays edge-pinned,
// so a conditional neighbour (e.g. a Report button that loads late) can't shift it under the cursor mid-click.
return (
<div className={cn('flex shrink-0 items-center gap-2 px-4', className)}>
{children}
{pager}
</div>
);
// The cluster is right-pinned, so it grows leftward: controls that come and go belong at the
// start of the children, and everything after them keeps its position when they appear.
export function AppHeaderActions({ children, className }: { children?: ReactNode; className?: string }) {
return <div className={cn('flex shrink-0 items-center gap-2 px-4', className)}>{children}</div>;
}
export function AppHeaderContent({ children, className }: { children: ReactNode; className?: string }) {
@@ -36,6 +36,9 @@ export function DetailNavigationButtons<T extends { id: string }>({
const isSm = size === 'sm';
const sideButtonSize = isSm ? 'size-7' : 'size-8';
const middleHeight = isSm ? 'h-7' : 'h-8';
// Reserve the widest label the set can produce ("2409/2409"), so stepping across a digit
// boundary cannot widen the counter and slide Previous out from under the cursor.
const reservedLabelWidth = `${String(controller.total).length * 2 + 1}ch`;
return (
<div className="flex items-center">
@@ -65,6 +68,7 @@ export function DetailNavigationButtons<T extends { id: string }>({
)}
disabled={!controller.hasEntries}
onClick={controller.openSheet}
style={{ minWidth: reservedLabelWidth }}
type="button"
variant="outline"
>
@@ -80,10 +80,11 @@ const renderToolbar = (props: HarnessProps = {}) => {
};
describe('DetailNavigationToolbar', () => {
it('renders nothing when raw items is empty', () => {
it('renders a disabled cluster when raw items is empty', () => {
renderToolbar({ items: [] });
expect(screen.queryByRole('button', { name: /Previous/i })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: /Next/i })).not.toBeInTheDocument();
expect(screen.getByRole('button', { name: /Previous/i })).toBeDisabled();
expect(screen.getByRole('button', { name: /Next/i })).toBeDisabled();
expect(screen.getByRole('button', { name: /\/0/ })).toBeDisabled();
});
it('composes Buttons + Sheet: position button opens the listbox', async () => {
@@ -25,9 +25,6 @@ export interface DetailNavigationToolbarProps<T extends { id: string }> {
* Most desktop call sites use this directly; pages with non-standard chrome
* (e.g. mobile prev/position/next inside a `<DropdownMenuItem>`) can compose
* the leaves themselves and read from the same controller.
*
* Renders `null` when the controller reports `itemsEmpty` — saves the user
* from a momentary "/0" flash while the parent provider's data is in flight.
*/
export function DetailNavigationToolbar<T extends { id: string }>({
controller,
@@ -37,10 +34,6 @@ export function DetailNavigationToolbar<T extends { id: string }>({
sheetIcon,
sheetTitle,
}: DetailNavigationToolbarProps<T>) {
if (controller.itemsEmpty) {
return null;
}
return (
<>
<DetailNavigationButtons
@@ -1,6 +1,6 @@
import type { ReactNode } from 'react';
import { Ellipsis, HatGlasses, LibraryBig, Pencil, Trash } from 'lucide-react';
import { Ellipsis, HatGlasses, LibraryBig, Pencil, Save, Trash } from 'lucide-react';
import { useCallback, useState } from 'react';
import { useNavigate } from 'react-router-dom';
import { toast } from 'sonner';
@@ -40,6 +40,7 @@ interface KnowledgeHeaderProps {
canAnonymize?: boolean;
isAnonymizeDisabled?: boolean;
isAnonymizing?: boolean;
isLoading?: boolean;
isNew: boolean;
knowledge?: KnowledgeDocumentFragmentFragment | null;
/**
@@ -71,6 +72,7 @@ export function KnowledgeHeader({
canAnonymize = false,
isAnonymizeDisabled = false,
isAnonymizing = false,
isLoading = false,
isNew,
knowledge,
onAnonymize,
@@ -95,9 +97,11 @@ export function KnowledgeHeader({
// header writes through `renameKnowledge`, which refreshes `knowledge` via
// the cache, and the form picks up the new value separately.
const knowledgeName = knowledge?.question ?? null;
const canShowActions = !isNew && !!knowledge;
const hasKnowledge = !!knowledge;
const isEntityPending = isLoading || !hasKnowledge;
const hasAnonymizeRow = isMobile && canAnonymize;
const hasNavRow = isMobile && knowledgeNav.total > 0;
const hasEntityRows = !isNew;
const hasNavRow = isMobile && !isNew;
const {
handleDropdownCloseAutoFocus,
@@ -161,7 +165,7 @@ export function KnowledgeHeader({
<BreadcrumbList className="min-w-0 flex-nowrap">
<BreadcrumbItem className="min-w-0 gap-2">
<LibraryBig className="size-4 shrink-0" />
{isEditingTitle && canShowActions ? (
{isEditingTitle && hasKnowledge ? (
<InlineEditInput
busy={isRenaming}
className="w-64 max-w-full min-w-0 flex-1"
@@ -171,7 +175,7 @@ export function KnowledgeHeader({
onSave={handleRenameSave}
placeholder="Knowledge question"
/>
) : canShowActions ? (
) : hasKnowledge ? (
<Tooltip>
<TooltipTrigger asChild>
<BreadcrumbPage
@@ -192,19 +196,7 @@ export function KnowledgeHeader({
</BreadcrumbList>
</Breadcrumb>
</AppHeaderContent>
<AppHeaderActions
pager={
canShowActions &&
!isMobile && (
<DetailNavigationToolbar<Knowledge>
controller={knowledgeNav}
renderItem={renderKnowledgeItem}
sheetIcon={<LibraryBig className="size-4" />}
sheetTitle="Knowledges"
/>
)
}
>
<AppHeaderActions>
{canAnonymize && !isMobile && (
<AppHeaderAction
disabled={isAnonymizeDisabled}
@@ -215,115 +207,131 @@ export function KnowledgeHeader({
variant="outline"
/>
)}
{saveButton}
{(canShowActions || (isMobile && canAnonymize) || !!onModeChange) && (
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
aria-label="Knowledge actions"
className="size-8 p-0"
type="button"
variant="ghost"
>
<Ellipsis />
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent
align="end"
className="min-w-24"
onCloseAutoFocus={handleDropdownCloseAutoFocus}
{saveButton ?? (
<AppHeaderAction
disabled
icon={<Save />}
label={isNew ? 'Create' : 'Save'}
type="button"
/>
)}
{!isNew && !isMobile && (
<DetailNavigationToolbar<Knowledge>
controller={knowledgeNav}
renderItem={renderKnowledgeItem}
sheetIcon={<LibraryBig className="size-4" />}
sheetTitle="Knowledges"
/>
)}
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
aria-label="Knowledge actions"
className="size-8 p-0"
type="button"
variant="ghost"
>
{hasAnonymizeRow && (
<Ellipsis />
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent
align="end"
className="min-w-24"
onCloseAutoFocus={handleDropdownCloseAutoFocus}
>
{hasAnonymizeRow && (
<DropdownMenuItem
disabled={isAnonymizeDisabled}
onClick={onAnonymize}
>
{isAnonymizing ? (
<>
<Spinner variant="circle" />
Anonymizing...
</>
) : (
<>
<HatGlasses />
Anonymize
</>
)}
</DropdownMenuItem>
)}
{hasNavRow && (
<>
{hasAnonymizeRow && <DropdownMenuSeparator />}
<DropdownMenuItem
disabled={isAnonymizeDisabled}
onClick={onAnonymize}
className="cursor-default hover:bg-transparent focus:bg-transparent"
onSelect={(event) => event.preventDefault()}
>
{isAnonymizing ? (
<LibraryBig />
Knowledges
<div className="-my-1.5 -mr-2 ml-auto flex items-center">
<DetailNavigationButtons<Knowledge>
controller={knowledgeNav}
sheetTitle="Knowledges"
size="sm"
/>
</div>
</DropdownMenuItem>
</>
)}
{hasEntityRows && (
<>
{(hasAnonymizeRow || hasNavRow) && <DropdownMenuSeparator />}
<DropdownMenuItem
disabled={isEntityPending}
onClick={handleRenameStart}
>
<Pencil className="size-3" />
Rename
</DropdownMenuItem>
<DropdownMenuSeparator />
</>
)}
{onModeChange ? (
<>
{!hasEntityRows && (hasAnonymizeRow || hasNavRow) && <DropdownMenuSeparator />}
<DropdownMenuItem
className="cursor-default gap-4 hover:bg-transparent focus:bg-transparent"
onSelect={(event) => event.preventDefault()}
>
View
<EditorViewModeToggle
className="-my-1.5 -mr-2 ml-auto"
mode={viewMode}
onModeChange={onModeChange}
rawTooltip="Edit the raw markdown"
/>
</DropdownMenuItem>
</>
) : null}
{hasEntityRows && (
<>
<DropdownMenuSeparator />
<DropdownMenuItem
disabled={isDeleting || isEntityPending}
onClick={() => setIsDeleteDialogOpen(true)}
>
{isDeleting ? (
<>
<Spinner variant="circle" />
Anonymizing...
Deleting...
</>
) : (
<>
<HatGlasses />
Anonymize
<Trash />
Delete
</>
)}
</DropdownMenuItem>
)}
{hasNavRow && (
<>
{hasAnonymizeRow && <DropdownMenuSeparator />}
<DropdownMenuItem
className="cursor-default hover:bg-transparent focus:bg-transparent"
onSelect={(event) => event.preventDefault()}
>
<LibraryBig />
Knowledges
<div className="-my-1.5 -mr-2 ml-auto flex items-center">
<DetailNavigationButtons<Knowledge>
controller={knowledgeNav}
sheetTitle="Knowledges"
size="sm"
/>
</div>
</DropdownMenuItem>
</>
)}
{canShowActions && (
<>
{(hasAnonymizeRow || hasNavRow) && <DropdownMenuSeparator />}
<DropdownMenuItem onClick={handleRenameStart}>
<Pencil className="size-3" />
Rename
</DropdownMenuItem>
<DropdownMenuSeparator />
</>
)}
{onModeChange ? (
<>
{!canShowActions && (hasAnonymizeRow || hasNavRow) && <DropdownMenuSeparator />}
<DropdownMenuItem
className="cursor-default gap-4 hover:bg-transparent focus:bg-transparent"
onSelect={(event) => event.preventDefault()}
>
View
<EditorViewModeToggle
className="-my-1.5 -mr-2 ml-auto"
mode={viewMode}
onModeChange={onModeChange}
rawTooltip="Edit the raw markdown"
/>
</DropdownMenuItem>
</>
) : null}
{canShowActions && (
<>
<DropdownMenuSeparator />
<DropdownMenuItem
disabled={isDeleting}
onClick={() => setIsDeleteDialogOpen(true)}
>
{isDeleting ? (
<>
<Spinner variant="circle" />
Deleting...
</>
) : (
<>
<Trash />
Delete
</>
)}
</DropdownMenuItem>
</>
)}
</DropdownMenuContent>
</DropdownMenu>
)}
</>
)}
</DropdownMenuContent>
</DropdownMenu>
</AppHeaderActions>
</AppHeader>
{isMobile && canShowActions && (
{isMobile && !isNew && (
<DetailNavigationSheet<Knowledge>
controller={knowledgeNav}
renderItem={renderKnowledgeItem}
@@ -9,6 +9,7 @@ import { KnowledgeHeader } from './knowledge-header';
interface KnowledgeLayoutProps {
children: ReactNode;
className?: string;
isLoading?: boolean;
isNew: boolean;
knowledge?: KnowledgeDocumentFragmentFragment | null;
saveButton?: ReactNode;
@@ -20,10 +21,18 @@ interface KnowledgeLayoutProps {
* renders the header inline because the form must be the parent of every
* input.
*/
export function KnowledgeLayout({ children, className, isNew, knowledge, saveButton }: KnowledgeLayoutProps) {
export function KnowledgeLayout({
children,
className,
isLoading,
isNew,
knowledge,
saveButton,
}: KnowledgeLayoutProps) {
return (
<div className={cn('flex min-h-[100dvh] flex-col', className)}>
<KnowledgeHeader
isLoading={isLoading}
isNew={isNew}
knowledge={knowledge}
saveButton={saveButton}
@@ -76,6 +76,7 @@ function Knowledge() {
if (!isNew && !knowledge) {
return (
<KnowledgeLayout
isLoading={isLoadingKnowledge}
isNew={false}
knowledge={knowledge}
>
+28 -33
View File
@@ -241,9 +241,6 @@ function Template() {
const { isDesktop, isMobile } = useBreakpoint();
const isNew = templateId === 'new';
// Pass `null` while creating a new template — there is no "current item"
// to highlight, and the toolbar shouldn't render at all anyway (gated
// below by `canShowActions`).
const templateNav = useTemplateDetailNavigation(isNew ? null : templateId);
const [expandedPresetIndex, setExpandedPresetIndex] = useState<null | number>(null);
@@ -415,7 +412,8 @@ function Template() {
}
}, [pendingPreset, setValue]);
const canShowActions = !isNew && !!templateData?.flowTemplate;
const hasTemplate = !!templateData?.flowTemplate;
const isTemplatePending = !isNew && (isLoadingTemplate || !hasTemplate);
const pageHeader = (
<>
@@ -424,7 +422,7 @@ function Template() {
<Breadcrumb className="min-w-0 flex-1">
<BreadcrumbList className="min-w-0 flex-nowrap">
<BreadcrumbItem className="min-w-0 gap-2">
{isEditingTitle && canShowActions ? (
{isEditingTitle && hasTemplate ? (
<InlineEditInput
busy={isRenaming}
className="w-64 max-w-full min-w-0 flex-1"
@@ -434,7 +432,7 @@ function Template() {
onSave={handleTemplateRenameSave}
placeholder="Template title"
/>
) : canShowActions ? (
) : hasTemplate ? (
<Tooltip>
<TooltipTrigger asChild>
<BreadcrumbPage
@@ -455,27 +453,21 @@ function Template() {
</BreadcrumbList>
</Breadcrumb>
</AppHeaderContent>
<AppHeaderActions
pager={
canShowActions &&
!isMobile && (
<DetailNavigationToolbar<Template>
controller={templateNav}
renderItem={renderTemplateItem}
sheetIcon={<FileText className="size-4" />}
sheetTitle="Templates"
/>
)
}
>
{(isNew || !!templateData?.flowTemplate) && (
<AppHeaderAction
disabled={!isNew && !hasUnsavedChanges}
form="template-form"
icon={<Save />}
label={isNew ? 'Create' : 'Save'}
loading={isSaving}
type="submit"
<AppHeaderActions>
<AppHeaderAction
disabled={isTemplatePending || (!isNew && !hasUnsavedChanges)}
form="template-form"
icon={<Save />}
label={isNew ? 'Create' : 'Save'}
loading={isSaving}
type="submit"
/>
{!isNew && !isMobile && (
<DetailNavigationToolbar<Template>
controller={templateNav}
renderItem={renderTemplateItem}
sheetIcon={<FileText className="size-4" />}
sheetTitle="Templates"
/>
)}
<DropdownMenu>
@@ -493,9 +485,9 @@ function Template() {
className="min-w-24"
onCloseAutoFocus={handleDropdownCloseAutoFocus}
>
{canShowActions && (
{!isNew && (
<>
{isMobile && templateNav.total > 0 && (
{isMobile && (
<>
<DropdownMenuItem
className="cursor-default hover:bg-transparent focus:bg-transparent"
@@ -514,7 +506,10 @@ function Template() {
<DropdownMenuSeparator />
</>
)}
<DropdownMenuItem onClick={handleTemplateRenameStart}>
<DropdownMenuItem
disabled={isTemplatePending}
onClick={handleTemplateRenameStart}
>
<Pencil />
Rename
</DropdownMenuItem>
@@ -533,11 +528,11 @@ function Template() {
rawTooltip="Edit the raw template"
/>
</DropdownMenuItem>
{canShowActions && (
{!isNew && (
<>
<DropdownMenuSeparator />
<DropdownMenuItem
disabled={isDeleting}
disabled={isDeleting || isTemplatePending}
onClick={() => setIsDeleteDialogOpen(true)}
>
{isDeleting ? (
@@ -558,7 +553,7 @@ function Template() {
</DropdownMenu>
</AppHeaderActions>
</AppHeader>
{isMobile && canShowActions && (
{isMobile && !isNew && (
<DetailNavigationSheet<Template>
controller={templateNav}
renderItem={renderTemplateItem}