From 74beb4200fac3f85316b072451021ece4e3a8d04 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 4 Jun 2026 17:03:38 +0000 Subject: [PATCH] fix(webapp): don't reload runs list when toggling bulk action inspector Prevent runs list revalidation and loading states when toggling the bulk action inspector. Filter, pagination, and explicit refresh behavior are unchanged. --- .server-changes/runs-bulk-action-no-reload.md | 6 + .../route.tsx | 92 +++++----- .../shouldRevalidateRunsList.ts | 75 ++++++++ .../useRunsLiveReload.ts | 7 +- ...ectParam.env.$envParam.runs.bulkaction.tsx | 19 +-- apps/webapp/app/utils/pathBuilder.ts | 3 +- .../test/shouldRevalidateRunsList.test.ts | 160 ++++++++++++++++++ 7 files changed, 306 insertions(+), 56 deletions(-) create mode 100644 .server-changes/runs-bulk-action-no-reload.md create mode 100644 apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/shouldRevalidateRunsList.ts create mode 100644 apps/webapp/test/shouldRevalidateRunsList.test.ts diff --git a/.server-changes/runs-bulk-action-no-reload.md b/.server-changes/runs-bulk-action-no-reload.md new file mode 100644 index 00000000000..1926ab20b75 --- /dev/null +++ b/.server-changes/runs-bulk-action-no-reload.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Stop reloading the runs list when opening or closing the bulk action inspector diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx index 3dff2bcd335..25cccaede8e 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx @@ -1,7 +1,7 @@ import { BeakerIcon, BookOpenIcon } from "@heroicons/react/24/solid"; -import { type MetaFunction, useNavigation, useRevalidator } from "@remix-run/react"; +import { type MetaFunction, useLocation, useNavigation, useRevalidator } from "@remix-run/react"; import { type LoaderFunctionArgs } from "@remix-run/server-runtime"; -import { Suspense } from "react"; +import { Suspense, useState } from "react"; import { TypedAwait, typeddefer, @@ -56,7 +56,6 @@ import { cn } from "~/utils/cn"; import { docsPath, EnvironmentParamSchema, - v3CreateBulkActionPath, v3ProjectPath, v3TestPath, v3TestTaskPath, @@ -65,8 +64,11 @@ import { throwNotFound } from "~/utils/httpErrors"; import { ListPagination } from "../../components/ListPagination"; import { CreateBulkActionInspector } from "../resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction"; import { Callout } from "~/components/primitives/Callout"; +import { isRunsListLoading, RUNS_BULK_INSPECTOR_OPEN_VALUE, shouldRevalidateRunsList } from "./shouldRevalidateRunsList"; import { useRunsLiveReload } from "./useRunsLiveReload"; +export { shouldRevalidateRunsList as shouldRevalidate }; + export const meta: MetaFunction = () => { return [ { @@ -209,8 +211,9 @@ function RunsList({ filters: TaskRunListSearchFilters; }) { const revalidator = useRevalidator(); + const location = useLocation(); const navigation = useNavigation(); - const isLoading = navigation.state !== "idle"; + const isLoading = isRunsListLoading(navigation, location.search); const organization = useOrganization(); const project = useProject(); const environment = useEnvironment(); @@ -244,7 +247,7 @@ function RunsList({ shortcut: { key: "r" }, action: (e) => { replace({ - bulkInspector: "true", + bulkInspector: RUNS_BULK_INSPECTOR_OPEN_VALUE, action: "replay", mode: selectedItems.size > 0 ? "selected" : undefined, }); @@ -254,7 +257,7 @@ function RunsList({ shortcut: { key: "c" }, action: (e) => { replace({ - bulkInspector: "true", + bulkInspector: RUNS_BULK_INSPECTOR_OPEN_VALUE, action: "cancel", mode: selectedItems.size > 0 ? "selected" : undefined, }); @@ -262,6 +265,13 @@ function RunsList({ }); const isShowingBulkActionInspector = has("bulkInspector") && list.hasAnyRuns; + const [isBulkInspectorPanelCollapsed, setIsBulkInspectorPanelCollapsed] = useState( + !isShowingBulkActionInspector + ); + // Keep content mounted until onCollapseChange reports the panel is fully collapsed. + const showBulkInspectorContent = + isShowingBulkActionInspector || !isBulkInspectorPanelCollapsed; + return ( @@ -310,39 +320,41 @@ function RunsList({ )} - {!isShowingBulkActionInspector && ( - 0 ? "selected" : undefined - )} - LeadingIcon={ListCheckedIcon} - className={selectedItems.size > 0 ? "pr-1" : undefined} - tooltip={ -
-
- Replay - -
-
- Cancel - -
+ {/* Stay mounted while the inspector is open to avoid toolbar layout shift. */} +
@@ -374,12 +386,12 @@ function RunsList({ className="overflow-hidden" collapsible collapsed={!isShowingBulkActionInspector} - onCollapseChange={() => {}} + onCollapseChange={setIsBulkInspectorPanelCollapsed} collapsedSize="0px" collapseAnimation={RESIZABLE_PANEL_ANIMATION} >
- {isShowingBulkActionInspector && ( + {showBulkInspectorContent && ( { + if (currentUrl.pathname !== nextUrl.pathname) { + return defaultShouldRevalidate; + } + + const currentParams = new URLSearchParams(currentUrl.search); + const nextParams = new URLSearchParams(nextUrl.search); + + if (currentParams.toString() === nextParams.toString()) { + return defaultShouldRevalidate; + } + + if (searchParamsEqualIgnoringBulkInspectorUiState(currentParams, nextParams)) { + return false; + } + + return defaultShouldRevalidate; +}; diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/useRunsLiveReload.ts b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/useRunsLiveReload.ts index fb27eb1f8cb..3ba1c972479 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/useRunsLiveReload.ts +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/useRunsLiveReload.ts @@ -4,8 +4,9 @@ import { useTypedFetcher } from "remix-typedjson"; import { useInterval } from "~/hooks/useInterval"; import type { NextRunListItem } from "~/presenters/v3/NextRunListPresenter.server"; import type { loader as liveRunsLoader } from "../resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.live"; +import { stripBulkInspectorUiParams } from "./shouldRevalidateRunsList"; -const RUNS_SEARCH_PARAMS_TO_REMOVE = ["cursor", "direction", "bulkInspector", "action", "mode"]; +const RUNS_PAGINATION_SEARCH_PARAMS = ["cursor", "direction"] as const; const RUNS_POLL_INTERVAL_MS = 3000; /** Check for new runs every N poll ticks (~6s at 3s interval). */ const NEW_RUNS_EVERY_N_POLL_TICKS = 2; @@ -30,8 +31,8 @@ function maxCreatedAtMs(runs: ListedRun[]): number | undefined { } function filterParamsWithoutPagination(search: string) { - const params = new URLSearchParams(search); - for (const key of RUNS_SEARCH_PARAMS_TO_REMOVE) { + const params = stripBulkInspectorUiParams(new URLSearchParams(search)); + for (const key of RUNS_PAGINATION_SEARCH_PARAMS) { params.delete(key); } return params; diff --git a/apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx b/apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx index be9a2763752..ab216bcab7e 100644 --- a/apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx +++ b/apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx @@ -23,7 +23,7 @@ import { AccordionItem, AccordionTrigger, } from "~/components/primitives/Accordion"; -import { Button, LinkButton } from "~/components/primitives/Buttons"; +import { Button } from "~/components/primitives/Buttons"; import { CheckboxWithLabel } from "~/components/primitives/Checkbox"; import { Dialog, @@ -51,10 +51,11 @@ import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/m import { findProjectBySlug } from "~/models/project.server"; import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server"; import { CreateBulkActionPresenter } from "~/presenters/v3/CreateBulkActionPresenter.server"; +import { RUNS_BULK_INSPECTOR_UI_SEARCH_PARAMS } from "~/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/shouldRevalidateRunsList"; import { logger } from "~/services/logger.server"; import { requireUserId } from "~/services/session.server"; import { cn } from "~/utils/cn"; -import { EnvironmentParamSchema, v3BulkActionPath, v3RunsPath } from "~/utils/pathBuilder"; +import { EnvironmentParamSchema, v3BulkActionPath } from "~/utils/pathBuilder"; import { BulkActionService } from "~/v3/services/bulk/BulkActionV2.server"; export async function loader({ request, params }: LoaderFunctionArgs) { @@ -187,7 +188,7 @@ export function CreateBulkActionInspector({ const project = useProject(); const environment = useEnvironment(); const fetcher = useTypedFetcher(); - const { value, replace } = useSearchParams(); + const { value, replace, del } = useSearchParams(); const [action, setAction] = useState( bulkActionActionFromString(value("action")) ); @@ -208,9 +209,6 @@ export function CreateBulkActionInspector({ const data = fetcher.data != null ? fetcher.data : undefined; - const closedSearchParams = new URLSearchParams(location.search); - closedSearchParams.delete("bulkInspector"); - const impactedCountElement = mode === "selected" ? selectedItems.size : ; @@ -225,13 +223,10 @@ export function CreateBulkActionInspector({
Create a bulk action - del([...RUNS_BULK_INSPECTOR_UI_SEARCH_PARAMS])} TrailingIcon={ExitIcon} shortcut={{ key: "esc" }} shortcutPosition="before-trailing-icon" diff --git a/apps/webapp/app/utils/pathBuilder.ts b/apps/webapp/app/utils/pathBuilder.ts index 0ebae151d0e..76254797669 100644 --- a/apps/webapp/app/utils/pathBuilder.ts +++ b/apps/webapp/app/utils/pathBuilder.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import { type TaskRunListSearchFilters } from "~/components/runs/v3/RunFilters"; import type { Organization } from "~/models/organization.server"; import type { Project } from "~/models/project.server"; +import { RUNS_BULK_INSPECTOR_OPEN_VALUE } from "~/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/shouldRevalidateRunsList"; import { objectToSearchParams } from "./searchParams"; import { type WaitpointSearchParams } from "~/components/runs/v3/WaitpointTokenFilters"; export type OrgForPath = Pick; @@ -363,7 +364,7 @@ export function v3CreateBulkActionPath( action?: "replay" | "cancel" ) { const searchParams = objectToSearchParams(filters) ?? new URLSearchParams(); - searchParams.set("bulkInspector", "show"); + searchParams.set("bulkInspector", RUNS_BULK_INSPECTOR_OPEN_VALUE); if (mode) { searchParams.set("mode", mode); } diff --git a/apps/webapp/test/shouldRevalidateRunsList.test.ts b/apps/webapp/test/shouldRevalidateRunsList.test.ts new file mode 100644 index 00000000000..2274ddddd21 --- /dev/null +++ b/apps/webapp/test/shouldRevalidateRunsList.test.ts @@ -0,0 +1,160 @@ +import type { Location, Navigation, ShouldRevalidateFunction } from "@remix-run/react"; +import { describe, expect, it } from "vitest"; +import { + isRunsListLoading, + RUNS_BULK_INSPECTOR_OPEN_VALUE, + shouldRevalidateRunsList, +} from "~/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/shouldRevalidateRunsList"; + +const runsUrl = (search: string) => + new URL(`http://localhost:3030/orgs/acme/projects/proj/env/dev/runs${search}`); + +function args( + currentSearch: string, + nextSearch: string, + defaultShouldRevalidate = true +): Parameters[0] { + return { + currentUrl: runsUrl(currentSearch), + nextUrl: runsUrl(nextSearch), + defaultShouldRevalidate, + formMethod: undefined, + formAction: undefined, + formData: undefined, + json: undefined, + actionResult: undefined, + }; +} + +describe("shouldRevalidateRunsList", () => { + it("returns false when only bulk inspector UI params change", () => { + expect( + shouldRevalidateRunsList( + args("?tasks=hello", `?tasks=hello&bulkInspector=${RUNS_BULK_INSPECTOR_OPEN_VALUE}&action=replay&mode=selected`) + ) + ).toBe(false); + }); + + it("returns false when closing the bulk inspector", () => { + expect( + shouldRevalidateRunsList( + args(`?tasks=hello&bulkInspector=${RUNS_BULK_INSPECTOR_OPEN_VALUE}`, "?tasks=hello") + ) + ).toBe(false); + }); + + it("returns false when list-data params are reordered", () => { + expect( + shouldRevalidateRunsList(args("?tasks=a&runtime=b", "?runtime=b&tasks=a")) + ).toBe(false); + }); + + it("returns default when list filters and bulk inspector UI params change together", () => { + expect( + shouldRevalidateRunsList(args("?tasks=a", `?tasks=b&bulkInspector=${RUNS_BULK_INSPECTOR_OPEN_VALUE}`)) + ).toBe(true); + }); + + it("returns default when list filters change", () => { + expect(shouldRevalidateRunsList(args("?tasks=hello", "?tasks=world"))).toBe(true); + }); + + it("returns default when pagination params change", () => { + expect( + shouldRevalidateRunsList( + args("?tasks=hello", "?tasks=hello&cursor=abc&direction=forward") + ) + ).toBe(true); + }); + + it("returns default when the URL is unchanged (explicit revalidate)", () => { + expect(shouldRevalidateRunsList(args("?tasks=hello", "?tasks=hello"))).toBe(true); + }); + + it("returns default when pathname changes", () => { + expect( + shouldRevalidateRunsList({ + ...args("?tasks=hello", "?tasks=hello"), + nextUrl: new URL("http://localhost:3030/orgs/acme/projects/proj/env/dev/tasks"), + }) + ).toBe(true); + }); + + it("respects defaultShouldRevalidate when false", () => { + expect( + shouldRevalidateRunsList(args("?tasks=hello", "?tasks=world", false)) + ).toBe(false); + }); +}); + +function makeLocation(search: string): Location { + const url = runsUrl(search); + return { + pathname: url.pathname, + search: url.search, + hash: url.hash, + state: null, + key: "test-key", + }; +} + +function navigation( + state: Navigation["state"], + nextSearch?: string +): Navigation { + return { + state, + location: nextSearch ? makeLocation(nextSearch) : undefined, + formMethod: undefined, + formAction: undefined, + formEncType: undefined, + formData: undefined, + json: undefined, + }; +} + +describe("isRunsListLoading", () => { + it("returns false when navigation is idle", () => { + expect(isRunsListLoading(navigation("idle"), "?tasks=hello")).toBe(false); + }); + + it("returns false when only bulk inspector UI params change", () => { + expect( + isRunsListLoading( + navigation("loading", `?tasks=hello&bulkInspector=${RUNS_BULK_INSPECTOR_OPEN_VALUE}&action=replay`), + "?tasks=hello" + ) + ).toBe(false); + }); + + it("returns false when list-data params are reordered", () => { + expect( + isRunsListLoading(navigation("loading", "?runtime=b&tasks=a"), "?tasks=a&runtime=b") + ).toBe(false); + }); + + it("returns true when list filters and bulk inspector UI params change together", () => { + expect( + isRunsListLoading(navigation("loading", `?tasks=b&bulkInspector=${RUNS_BULK_INSPECTOR_OPEN_VALUE}`), "?tasks=a") + ).toBe(true); + }); + + it("returns true when list filters change", () => { + expect( + isRunsListLoading(navigation("loading", "?tasks=world"), "?tasks=hello") + ).toBe(true); + }); + + it("returns true when pagination params change", () => { + expect( + isRunsListLoading( + navigation("loading", "?tasks=hello&cursor=abc&direction=forward"), + "?tasks=hello" + ) + ).toBe(true); + }); + + it("returns false when navigation is loading without a target location", () => { + expect(isRunsListLoading(navigation("loading"), "?tasks=hello")).toBe(false); + }); +});