From 15319ef817d761d3e58e00c4ba0932a6bef2c1e6 Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Wed, 22 Jul 2026 16:21:38 +0700 Subject: [PATCH] fix(ui): reserve the counter width on the label, not on the button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reservation was computed as digits-of-total × 2 + 1 and applied to the button, which is border-box: 18px of padding and border ate it, so the counter still grew from 65 to 83px across a digit boundary and Previous still slid 18px out from under the cursor — measured, both before and after. It now reserves the width of the widest label the set can produce, on the label itself, and the button holds 83px through every position. Building that label instead of deriving its length also drops the arithmetic that made the intent unreadable. Co-Authored-By: Claude Opus 4.8 --- frontend/e2e/specs/cross/responsive.spec.ts | 4 ++-- frontend/e2e/specs/flows/pager.spec.ts | 9 ++------- .../detail-navigation/detail-navigation-buttons.tsx | 9 ++++----- frontend/src/providers/flow-provider.tsx | 8 +++----- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/frontend/e2e/specs/cross/responsive.spec.ts b/frontend/e2e/specs/cross/responsive.spec.ts index 6693e5fb..f22344d0 100644 --- a/frontend/e2e/specs/cross/responsive.spec.ts +++ b/frontend/e2e/specs/cross/responsive.spec.ts @@ -24,8 +24,8 @@ 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. + // The only load gate left here: below md the star is inside the menu, and the + // actions trigger 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); diff --git a/frontend/e2e/specs/flows/pager.spec.ts b/frontend/e2e/specs/flows/pager.spec.ts index 5da9dab5..f7bc95d2 100644 --- a/frontend/e2e/specs/flows/pager.spec.ts +++ b/frontend/e2e/specs/flows/pager.spec.ts @@ -2,8 +2,6 @@ import { expect, test } from '../../fixtures/test.ts'; import { expectCleanPage } from '../../helpers/errors.ts'; import { flowsCassette, flowTabsCassette } from '../../mocks/cassettes/flows.ts'; -// Nothing else in the suite presses Prev/Next, which is how a redirect that fired mid-switch -// shipped: every other flow spec reaches a detail page through the list. test.describe('flow pager', { tag: ['@flows', '@smoke'] }, () => { test.use({ cassette: flowsCassette() }); @@ -21,8 +19,6 @@ test.describe('flow pager', { tag: ['@flows', '@smoke'] }, () => { .evaluateAll((buttons) => buttons.map((button) => button.getAttribute('aria-label') ?? '')); const positionOf = (label: string) => labels.findIndex((candidate) => candidate.startsWith(label)); - // Report comes and goes with the task list; anchored leftmost, its arrival cannot shift - // the controls that are always there. expect(positionOf('Report')).toBeLessThan(positionOf('Toggle favorite')); expect(positionOf('Toggle favorite')).toBeLessThan(positionOf('Previous')); expect(positionOf('Next')).toBeLessThan(positionOf('Flow actions')); @@ -61,9 +57,8 @@ test.describe('flow pager', { tag: ['@flows', '@smoke'] }, () => { await expect(page).toHaveURL(/\/flows\/6$/); await expect(header.getByText('E2E Beta')).toBeVisible(); - // Sampled rather than awaited: `toBeVisible` retries, so it passes even if the cluster - // unmounts for the length of the fetch and comes back — which is what gating the pager - // on the loaded flow did, costing the user a round trip per step. + // Sampled, not awaited: `toBeVisible` retries, so it passes even if the cluster + // unmounts for the length of the fetch and comes back. expect(samples.every((sample) => sample.hasPager)).toBe(true); expect(samples.map((sample) => sample.path)).not.toContain('/flows'); diff --git a/frontend/src/components/shared/detail-navigation/detail-navigation-buttons.tsx b/frontend/src/components/shared/detail-navigation/detail-navigation-buttons.tsx index 692811e0..10785d7c 100644 --- a/frontend/src/components/shared/detail-navigation/detail-navigation-buttons.tsx +++ b/frontend/src/components/shared/detail-navigation/detail-navigation-buttons.tsx @@ -36,9 +36,7 @@ export function DetailNavigationButtons({ 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`; + const widestLabel = `${controller.total}/${controller.total}`; return (
@@ -68,11 +66,12 @@ export function DetailNavigationButtons({ )} disabled={!controller.hasEntries} onClick={controller.openSheet} - style={{ minWidth: reservedLabelWidth }} type="button" variant="outline" > - {controller.positionLabel} + {/* Holding the widest label's width keeps Previous still when stepping past + a digit boundary; on the button itself the reserve would go to padding. */} + {controller.positionLabel} Show all matching {lowerTitle} diff --git a/frontend/src/providers/flow-provider.tsx b/frontend/src/providers/flow-provider.tsx index db4a435a..9f31249d 100644 --- a/frontend/src/providers/flow-provider.tsx +++ b/frontend/src/providers/flow-provider.tsx @@ -85,9 +85,8 @@ export function FlowProvider({ children }: FlowProviderProps) { variables: { id: flowId ?? '' }, }); - // In flight with nothing to show. A background refetch (the reconnect reconcile) still - // holds the previous flow, so it must not raise this: it would cover the page with the - // spinner overlay and tear down the 14 live subscriptions mid-flight. + // A refetch that still holds the previous flow must not raise this: it drives the spinner + // overlay and `subscriptionSkip`, so it would tear down 14 live subscriptions mid-flight. const isLoading = loading && !flowData?.flow; // A real load failure that left nothing to show (cold cache + backend error on a @@ -95,8 +94,7 @@ export function FlowProvider({ children }: FlowProviderProps) { // in-page ErrorState + Retry instead of silently bouncing to the list. const flowLoadError = flowError && !flowData?.flow && !isFlowNotFoundError(flowError) ? flowError : undefined; - // Absence of data is not absence of the flow — it is also what an in-flight switch to - // another flow looks like, so this reads only settled outcomes. + // Settled outcomes only: missing data is also what an in-flight switch looks like. const isFlowMissing = Boolean(flowData && !flowData.flow) || Boolean(flowError && isFlowNotFoundError(flowError)); const { data: assistantsData, loading: isAssistantsLoading } = useQuery(AssistantsDocument, {