mirror of
https://github.com/vxcontrol/pentagi.git
synced 2026-08-26 13:06:32 +00:00
fix(ui): reserve the counter width on the label, not on the button
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
ee6ed49d48
commit
15319ef817
@@ -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);
|
||||
|
||||
@@ -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');
|
||||
|
||||
|
||||
@@ -36,9 +36,7 @@ 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`;
|
||||
const widestLabel = `${controller.total}/${controller.total}`;
|
||||
|
||||
return (
|
||||
<div className="flex items-center">
|
||||
@@ -68,11 +66,12 @@ export function DetailNavigationButtons<T extends { id: string }>({
|
||||
)}
|
||||
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. */}
|
||||
<span style={{ minWidth: `${widestLabel.length}ch` }}>{controller.positionLabel}</span>
|
||||
</Button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent>Show all matching {lowerTitle}</TooltipContent>
|
||||
|
||||
@@ -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, {
|
||||
|
||||
Reference in New Issue
Block a user