diff --git a/frontend/e2e/specs/flows/pager.spec.ts b/frontend/e2e/specs/flows/pager.spec.ts new file mode 100644 index 00000000..5da9dab5 --- /dev/null +++ b/frontend/e2e/specs/flows/pager.spec.ts @@ -0,0 +1,75 @@ +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() }); + + test.describe('with a report to show', () => { + test.use({ cassette: flowTabsCassette() }); + + // Not covered by the pixel baselines: the header cluster is well under the visual + // project's maxDiffPixelRatio, so a reordering passes there unnoticed. + test('keeps the variable action left of the fixed ones', async ({ page }) => { + await page.goto('/flows/5'); + await expect(page.locator('header').getByRole('button', { name: 'Report' })).toBeVisible(); + + const labels = await page + .locator('header button') + .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')); + }); + }); + + test('steps to the sibling flow and back without passing through the list', async ({ page, pageErrorLog }) => { + const header = page.locator('header'); + + await page.goto('/flows/5'); + await expect(header.getByText('E2E Alpha')).toBeVisible(); + + // Hold the sibling's data back so the in-flight window is wide enough to sample: the + // cassette answers in single-digit milliseconds, which hides an unmount entirely. + await page.route('**/graphql', async (route) => { + await new Promise((resolve) => setTimeout(resolve, 300)); + await route.fallback(); + }); + + await header.getByRole('button', { name: 'Next' }).click(); + + const samples = await page.evaluate(async () => { + const taken = []; + + for (let index = 0; index < 10; index += 1) { + taken.push({ + hasPager: !!document.querySelector('header button[aria-label="Next"]'), + path: window.location.pathname, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + } + + return taken; + }); + + 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. + expect(samples.every((sample) => sample.hasPager)).toBe(true); + expect(samples.map((sample) => sample.path)).not.toContain('/flows'); + + await header.getByRole('button', { name: 'Previous' }).click(); + + await expect(page).toHaveURL(/\/flows\/5$/); + expectCleanPage(pageErrorLog); + }); +}); diff --git a/frontend/src/pages/flows/flow.tsx b/frontend/src/pages/flows/flow.tsx index 36eba66c..5996a649 100644 --- a/frontend/src/pages/flows/flow.tsx +++ b/frontend/src/pages/flows/flow.tsx @@ -80,7 +80,7 @@ function Flow() { const { isDesktop, isMobile } = useBreakpoint(); const navigate = useNavigate(); - const { flowData, flowId, flowLoadError, isLoading: isFlowLoading, refetchFlow } = useFlow(); + const { flowData, flowId, flowLoadError, isFlowMissing, isLoading: isFlowLoading, refetchFlow } = useFlow(); const { deleteFlow, finishFlow } = useFlows(); const { isFavoriteFlow, toggleFavoriteFlow } = useFavorites(); @@ -109,13 +109,10 @@ function Flow() { const [renameFlowMutation, { loading: isRenameLoading }] = useMutation(RenameFlowDocument); useEffect(() => { - // Redirect only when the flow is genuinely gone (query resolved with no flow, or - // a not-found error). A real load failure keeps the user here behind the - // ErrorState + Retry below instead of silently bouncing to the list. - if (!isFlowLoading && !flowData?.flow && !flowLoadError) { + if (isFlowMissing) { navigate(routes.flows, { replace: true }); } - }, [flowData, flowLoadError, isFlowLoading, navigate]); + }, [isFlowMissing, navigate]); const handleFlowRenameSave = useCallback(async () => { const newTitle = editingInputRef.current?.value.trim(); @@ -278,24 +275,14 @@ function Flow() { - - controller={flowNav} - renderItem={renderFlowItem} - sheetIcon={} - sheetTitle="Flows" - /> - ) - } - > + + {!!(flowData?.tasks ?? [])?.length && } {flowId && !isMobile && ( )} - {!!(flowData?.tasks ?? [])?.length && } - {flow && ( - - - - - + controller={flowNav} + renderItem={renderFlowItem} + sheetIcon={} + sheetTitle="Flows" + /> + )} + + + + + + {isMobile && ( + <> + {/* onSelect={preventDefault} stops the Radix menu from closing on label clicks; DetailNavigationButtons owns its own click handlers. */} - event.preventDefault()} - > - - Flows -
- - controller={flowNav} - sheetTitle="Flows" - size="sm" - /> -
- - {flowId && ( - toggleFavoriteFlow(flowId)}> - - {isFavoriteFlow(flowId) ? 'Remove from favorites' : 'Add to favorites'} - - )} - - - )} - - - Rename - - {isFlowRunning && ( handleFlowFinish()} + className="cursor-default hover:bg-transparent focus:bg-transparent" + onSelect={(event) => event.preventDefault()} > - {isFinishing ? ( - <> - - Finishing... - - ) : ( - <> - - Finish - - )} + + Flows +
+ + controller={flowNav} + sheetTitle="Flows" + size="sm" + /> +
- )} - + {flowId && ( + toggleFavoriteFlow(flowId)} + > + + {isFavoriteFlow(flowId) ? 'Remove from favorites' : 'Add to favorites'} + + )} + + + )} + + + Rename + + {isFlowRunning && ( setIsDeleteDialogOpen(true)} + disabled={isFinishing} + onClick={() => handleFlowFinish()} > - {isDeleting ? ( + {isFinishing ? ( <> - Deleting... + Finishing... ) : ( <> - - Delete + + Finish )} - - - )} + )} + + setIsDeleteDialogOpen(true)} + > + {isDeleting ? ( + <> + + Deleting... + + ) : ( + <> + + Delete + + )} + + + - {isMobile && flow && ( + {isMobile && ( controller={flowNav} renderItem={renderFlowItem} diff --git a/frontend/src/providers/flow-provider.tsx b/frontend/src/providers/flow-provider.tsx index c66336ca..db4a435a 100644 --- a/frontend/src/providers/flow-provider.tsx +++ b/frontend/src/providers/flow-provider.tsx @@ -1,4 +1,3 @@ -import { NetworkStatus } from '@apollo/client'; import { useMutation, useQuery, useSubscription } from '@apollo/client/react'; import { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react'; import { useParams } from 'react-router-dom'; @@ -50,6 +49,7 @@ interface FlowContextValue { flowStatus: StatusType | undefined; initiateAssistantCreation: () => void; isAssistantsLoading: boolean; + isFlowMissing: boolean; isLoading: boolean; refetchFlow: () => void; selectAssistant: (assistantId: null | string) => void; @@ -75,7 +75,6 @@ export function FlowProvider({ children }: FlowProviderProps) { data: flowData, error: flowError, loading, - networkStatus, refetch: refetchFlow, } = useQuery(FlowDocument, { errorPolicy: 'all', @@ -86,17 +85,20 @@ export function FlowProvider({ children }: FlowProviderProps) { variables: { id: flowId ?? '' }, }); - // Only the initial load blocks the UI and gates subscriptions. A background - // refetch (e.g. the reconnect reconcile) stays at networkStatus `refetch`, so - // it must NOT flip isLoading — otherwise it covers the page with the spinner - // overlay and tears down the 14 live subscriptions mid-flight. - const isLoading = loading && networkStatus === NetworkStatus.loading; + // 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. + const isLoading = loading && !flowData?.flow; // A real load failure that left nothing to show (cold cache + backend error on a // deep link), as opposed to a genuine not-found. The detail page renders this as an // 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. + const isFlowMissing = Boolean(flowData && !flowData.flow) || Boolean(flowError && isFlowNotFoundError(flowError)); + const { data: assistantsData, loading: isAssistantsLoading } = useQuery(AssistantsDocument, { fetchPolicy: 'cache-first', nextFetchPolicy: 'cache-first', @@ -397,6 +399,7 @@ export function FlowProvider({ children }: FlowProviderProps) { flowStatus, initiateAssistantCreation, isAssistantsLoading, + isFlowMissing, isLoading, refetchFlow, selectAssistant, @@ -417,6 +420,7 @@ export function FlowProvider({ children }: FlowProviderProps) { flowStatus, initiateAssistantCreation, isAssistantsLoading, + isFlowMissing, isLoading, refetchFlow, selectAssistant,