Skip to content

Commit 20ca1c5

Browse files
ericallamericallam
committed
fix(webapp): address PR review on runs-list column customization
- Import ResolvedColumn in runColumns.test.ts (typecheck). - Populate payload/output smart columns on the per-task, scheduled, agent and error run lists by threading the column select through those loaders and ErrorGroupPresenter (previously only the main runs list did). - Memoize per-row source parsing so payload/output are decoded once per run rather than on every render / live-poll tick. - Support column reordering in Firefox (set drag data on dragstart, preventDefault on drop) and add keyboard reordering via the grip handle (arrow up/down), revealing row controls on focus. - Show the tags cell placeholder for an empty tag list, and let a live update clear a source value instead of keeping stale data. - Escape backslashes in bracket-notation sample paths.
1 parent 08734eb commit 20ca1c5

10 files changed

Lines changed: 79 additions & 16 deletions

File tree

‎apps/webapp/app/components/runs/v3/RunsDisplayOptions.tsx‎

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,16 @@ export function RunsDisplayOptions() {
109109
applyLayout(arr);
110110
};
111111

112+
const move = (key: string, delta: number) => {
113+
const arr = [...layout.ordered];
114+
const from = arr.findIndex((o) => keyFor(o.col) === key);
115+
const to = from + delta;
116+
if (from < 0 || to < 0 || to >= arr.length) return;
117+
const [moved] = arr.splice(from, 1);
118+
arr.splice(to, 0, moved);
119+
applyLayout(arr);
120+
};
121+
112122
const endDrag = () => {
113123
setDragKey(null);
114124
setOverKey(null);
@@ -148,6 +158,7 @@ export function RunsDisplayOptions() {
148158
endDrag();
149159
}}
150160
onToggle={() => toggleHidden(key)}
161+
onMove={(delta) => move(key, delta)}
151162
onEdit={
152163
col.kind === "smart"
153164
? () => setEditing({ index: col.index, def: col.def })
@@ -199,10 +210,10 @@ function ColumnRow({
199210
col,
200211
checked,
201212
locked,
202-
reserveIcon,
203213
dragging,
204214
isOver,
205215
onToggle,
216+
onMove,
206217
onEdit,
207218
onRemove,
208219
onDragStart,
@@ -216,6 +227,7 @@ function ColumnRow({
216227
dragging: boolean;
217228
isOver: boolean;
218229
onToggle: () => void;
230+
onMove: (delta: number) => void;
219231
onEdit?: () => void;
220232
onRemove?: () => void;
221233
onDragStart: () => void;
@@ -232,11 +244,18 @@ function ColumnRow({
232244
dragging && "opacity-40"
233245
)}
234246
draggable
235-
onDragStart={onDragStart}
247+
onDragStart={(e) => {
248+
e.dataTransfer.effectAllowed = "move";
249+
e.dataTransfer.setData("text/plain", "");
250+
onDragStart();
251+
}}
236252
onDragEnter={onDragEnter}
237253
onDragEnd={onDragEnd}
238254
onDragOver={(e) => e.preventDefault()}
239-
onDrop={onDrop}
255+
onDrop={(e) => {
256+
e.preventDefault();
257+
onDrop();
258+
}}
240259
>
241260
{isOver && <div className="absolute inset-x-0 top-0 h-0.5 bg-blue-500" />}
242261
{locked ? <Checkbox checked disabled /> : <Checkbox checked={checked} onChange={onToggle} />}
@@ -254,7 +273,7 @@ function ColumnRow({
254273
type="button"
255274
onClick={onEdit}
256275
aria-label={`Edit ${col.def.label}`}
257-
className="flex size-6 items-center justify-center rounded text-text-dimmed opacity-0 transition hover:bg-charcoal-700 hover:text-text-bright focus-custom group-hover:opacity-100"
276+
className="flex size-6 items-center justify-center rounded text-text-dimmed opacity-0 transition hover:bg-charcoal-700 hover:text-text-bright focus-custom group-hover:opacity-100 group-focus-within:opacity-100"
258277
>
259278
<PencilSquareIcon className="size-4" />
260279
</button>
@@ -264,14 +283,27 @@ function ColumnRow({
264283
type="button"
265284
onClick={onRemove}
266285
aria-label={`Remove ${col.def.label}`}
267-
className="flex size-6 items-center justify-center rounded text-text-dimmed opacity-0 transition hover:bg-charcoal-700 hover:text-error focus-custom group-hover:opacity-100"
286+
className="flex size-6 items-center justify-center rounded text-text-dimmed opacity-0 transition hover:bg-charcoal-700 hover:text-error focus-custom group-hover:opacity-100 group-focus-within:opacity-100"
268287
>
269288
<XMarkIcon className="size-4" />
270289
</button>
271290
)}
272-
<span className="flex size-6 cursor-grab items-center justify-center text-text-dimmed opacity-0 transition group-hover:opacity-100 active:cursor-grabbing">
291+
<button
292+
type="button"
293+
aria-label={`Reorder ${col.def.label} (use arrow up and down)`}
294+
onKeyDown={(e) => {
295+
if (e.key === "ArrowUp") {
296+
e.preventDefault();
297+
onMove(-1);
298+
} else if (e.key === "ArrowDown") {
299+
e.preventDefault();
300+
onMove(1);
301+
}
302+
}}
303+
className="flex size-6 cursor-grab items-center justify-center rounded text-text-dimmed opacity-0 transition hover:bg-charcoal-700 hover:text-text-bright focus-custom group-hover:opacity-100 group-focus-within:opacity-100 active:cursor-grabbing"
304+
>
273305
<GripVerticalIcon className="size-4" />
274-
</span>
306+
</button>
275307
</div>
276308
</div>
277309
);

‎apps/webapp/app/components/runs/v3/SmartColumnSample.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export function SmartColumnSample({
3535
function childPath(parentPath: string, key: string | number): string {
3636
if (typeof key === "number") return `${parentPath}[${key}]`;
3737
if (/^[A-Za-z_$][A-Za-z0-9_$]*$/.test(key)) return `${parentPath}.${key}`;
38-
return `${parentPath}['${key.replace(/'/g, "\\'")}']`;
38+
return `${parentPath}['${key.replace(/\\/g, "\\\\").replace(/'/g, "\\'")}']`;
3939
}
4040

4141
function JsonNode({

‎apps/webapp/app/components/runs/v3/TaskRunsTable.tsx‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ const STANDARD_RENDERERS: Record<string, StandardColumnRenderer> = {
472472
cell: ({ run, path }) => (
473473
<TableCell to={path} actionClassName="py-1" className="pr-16">
474474
<div className="flex gap-1">
475-
{run.tags.map((tag) => <RunTag key={tag} tag={tag} />) || "–"}
475+
{run.tags.length > 0 ? run.tags.map((tag) => <RunTag key={tag} tag={tag} />) : "–"}
476476
</div>
477477
</TableCell>
478478
),
@@ -511,6 +511,8 @@ function SmartColumnCell({
511511
);
512512
}
513513

514+
const EMPTY_SOURCES: Partial<Record<SmartColumnSource, ParsedSource>> = {};
515+
514516
function buildRowSources(
515517
run: NextRunListItem,
516518
sources: SmartColumnSource[]
@@ -611,6 +613,15 @@ export function TaskRunsTable({
611613
const visibleColumns = layout.visible;
612614
const referencedSources = useMemo(() => visibleSmartSources(visibleColumns), [visibleColumns]);
613615

616+
const sourcesByRunId = useMemo(() => {
617+
const map = new Map<string, Partial<Record<SmartColumnSource, ParsedSource>>>();
618+
if (referencedSources.length === 0) return map;
619+
for (const run of runs) {
620+
map.set(run.id, buildRowSources(run, referencedSources));
621+
}
622+
return map;
623+
}, [runs, referencedSources]);
624+
614625
const dataColSpan = visibleColumns.reduce(
615626
(sum, col) => sum + (col.kind === "standard" ? (STANDARD_RENDERERS[col.def.id]?.span ?? 1) : 1),
616627
0
@@ -705,7 +716,7 @@ export function TaskRunsTable({
705716
},
706717
searchParams
707718
);
708-
const sources = buildRowSources(run, referencedSources);
719+
const sources = sourcesByRunId.get(run.id) ?? EMPTY_SOURCES;
709720
return (
710721
<TableRow key={run.id}>
711722
{allowSelection && (

‎apps/webapp/app/components/runs/v3/runColumns.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
encodeColumnLayout,
77
encodeSmartColumn,
88
resolveColumnLayout,
9+
type ResolvedColumn,
910
type RunColumnRuntime,
1011
type SmartColumnDef,
1112
} from "./runColumns";

‎apps/webapp/app/presenters/v3/ErrorGroupPresenter.server.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ import {
1313
type NextRunList,
1414
} from "~/presenters/v3/NextRunListPresenter.server";
1515
import { sortVersionsDescending } from "~/utils/semver";
16+
import type { RunColumnId, SmartColumnSource } from "~/components/runs/v3/runColumns";
17+
18+
type RunColumnsSelect = {
19+
visibleStandardIds: RunColumnId[];
20+
smartSources: SmartColumnSource[];
21+
};
1622

1723
const errorGroupGranularity = new TimeGranularity([
1824
{ max: "1h", granularity: "1m" },
@@ -34,6 +40,7 @@ export type ErrorGroupOptions = {
3440
to?: number;
3541
cursor?: string;
3642
direction?: Direction;
43+
columns?: RunColumnsSelect;
3744
};
3845

3946
export const ErrorGroupOptionsSchema = z.object({
@@ -115,6 +122,7 @@ export class ErrorGroupPresenter extends BasePresenter {
115122
to,
116123
cursor,
117124
direction,
125+
columns,
118126
}: ErrorGroupOptions
119127
) {
120128
const displayableEnvironment = await findDisplayableEnvironment(environmentId, userId);
@@ -144,6 +152,7 @@ export class ErrorGroupPresenter extends BasePresenter {
144152
to: time.to.getTime(),
145153
cursor,
146154
direction,
155+
columns,
147156
}),
148157
this.getState(environmentId, summary?.taskIdentifier, fingerprint),
149158
]);
@@ -413,6 +422,7 @@ export class ErrorGroupPresenter extends BasePresenter {
413422
to?: number;
414423
cursor?: string;
415424
direction?: Direction;
425+
columns?: RunColumnsSelect;
416426
}
417427
): Promise<NextRunList | undefined> {
418428
const runListPresenter = new NextRunListPresenter(this.replica, this.clickhouse);
@@ -428,6 +438,7 @@ export class ErrorGroupPresenter extends BasePresenter {
428438
to: options.to,
429439
cursor: options.cursor,
430440
direction: options.direction,
441+
columns: options.columns,
431442
});
432443

433444
if (result.runs.length === 0) {

‎apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.agents.$agentParam/route.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
type AgentDetail,
3939
} from "~/presenters/v3/AgentDetailPresenter.server";
4040
import { NextRunListPresenter } from "~/presenters/v3/NextRunListPresenter.server";
41+
import { getRunColumnsForSelect } from "~/presenters/v3/runColumnsFromRequest.server";
4142
import { SessionListPresenter } from "~/presenters/v3/SessionListPresenter.server";
4243
import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server";
4344
import { getResizableSnapshot } from "~/services/resizablePanel.server";
@@ -162,6 +163,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
162163
to,
163164
cursor,
164165
direction,
166+
columns: getRunColumnsForSelect(request),
165167
})
166168
.catch(() => null);
167169

‎apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ import {
7474
type ErrorGroupSummary,
7575
} from "~/presenters/v3/ErrorGroupPresenter.server";
7676
import { type NextRunList } from "~/presenters/v3/NextRunListPresenter.server";
77+
import { getRunColumnsForSelect } from "~/presenters/v3/runColumnsFromRequest.server";
7778
import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server";
7879
import { requireUser, requireUserId } from "~/services/session.server";
7980
import { rbac } from "~/services/rbac.server";
@@ -268,6 +269,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
268269
to,
269270
cursor,
270271
direction,
272+
columns: getRunColumnsForSelect(request),
271273
})
272274
.catch((error) => {
273275
if (error instanceof ServiceValidationError) {

‎apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/useRunsLiveReload.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,12 @@ function patchVisibleRunsWithLiveUpdates(currentRuns: ListedRun[], liveRuns: Liv
8787
usageDurationMs: update.usageDurationMs,
8888
costInCents: update.costInCents,
8989
baseCostInCents: update.baseCostInCents,
90-
metadata: update.metadata ?? run.metadata,
91-
metadataType: update.metadataType ?? run.metadataType,
92-
payload: update.payload ?? run.payload,
93-
payloadType: update.payloadType ?? run.payloadType,
94-
output: update.output ?? run.output,
95-
outputType: update.outputType ?? run.outputType,
90+
metadata: update.metadata !== undefined ? update.metadata : run.metadata,
91+
metadataType: update.metadataType !== undefined ? update.metadataType : run.metadataType,
92+
payload: update.payload !== undefined ? update.payload : run.payload,
93+
payloadType: update.payloadType !== undefined ? update.payloadType : run.payloadType,
94+
output: update.output !== undefined ? update.output : run.output,
95+
outputType: update.outputType !== undefined ? update.outputType : run.outputType,
9696
};
9797
});
9898
}

‎apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.tasks.scheduled.$taskParam/route.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ import { useZoomToTimeFilter } from "~/hooks/useZoomToTimeFilter";
7575
import { findProjectBySlug } from "~/models/project.server";
7676
import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server";
7777
import { NextRunListPresenter } from "~/presenters/v3/NextRunListPresenter.server";
78+
import { getRunColumnsForSelect } from "~/presenters/v3/runColumnsFromRequest.server";
7879
import { ScheduleListPresenter } from "~/presenters/v3/ScheduleListPresenter.server";
7980
import {
8081
TaskDetailPresenter,
@@ -219,6 +220,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
219220
cursor,
220221
direction,
221222
includeHasAnyRuns: true,
223+
columns: getRunColumnsForSelect(request),
222224
})
223225
.catch(() => null);
224226

‎apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.tasks.standard.$taskParam/route.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import { useSearchParams } from "~/hooks/useSearchParam";
4646
import { findProjectBySlug } from "~/models/project.server";
4747
import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server";
4848
import { NextRunListPresenter } from "~/presenters/v3/NextRunListPresenter.server";
49+
import { getRunColumnsForSelect } from "~/presenters/v3/runColumnsFromRequest.server";
4950
import {
5051
TaskDetailPresenter,
5152
type TaskActivity,
@@ -163,6 +164,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
163164
cursor,
164165
direction,
165166
includeHasAnyRuns: true,
167+
columns: getRunColumnsForSelect(request),
166168
})
167169
.catch(() => null);
168170

0 commit comments

Comments
 (0)