improvement(data-drains): move drains to the fullscreen list/detail pattern by waleedlatif1 · Pull Request #5954 · simstudioai/sim

waleedlatif1 · 2026-07-25T02:33:37Z

Summary

Data drains was the last settings surface still on a Table + create modal. It now matches Skills and Custom Tools.
List is SettingsResourceRow rows in a clickable button — source, destination, cadence, and last run on the row, plus a Disabled tag when a drain is paused.
Clicking a row opens a detail sub-view with the drain's actions (Run now / Test connection / Delete), its enabled toggle, resolved destination config, and recent run history — replacing the expanding table row.
Creating a drain is a fullscreen view instead of a ChipModal, reusing DESTINATION_FORM_REGISTRY so the two never fork.
A drain is deep-linkable via data-drain-id (push to open, replace to close), matching custom-tool-id / custom-block-id.

The detail is read-only apart from the enabled toggle — destination credentials are never returned by the API, so changing one means recreating the drain.

Alignment work the migration surfaced:

The destination registry rendered its 36 fields with ChipModalField, whose label is byte-identical to a SettingsSection header (text-small, --text-muted, pl-0.5) — on a page that made section titles and field labels indistinguishable. All of them, plus the create view's own fields, now use SettingRow like every other page-level detail view. The registry had exactly one consumer, so no modal was left behind.
Shared source/destination/cadence label maps moved to labels.ts now that three files need them (verified byte-identical to the originals).
Run status uses Badge with a dot instead of hand-coloured text, and byte counts use formatFileSize — the old /1024 math rendered a 5 GB export as 5242880.0 KB.
Copy follows the sibling surfaces: Create drain (not New drain), Disabled (not Paused), Run queued, Connection test passed, No drains found matching ….
Seed the list cache on create so landing on the new drain's detail doesn't depend on the invalidation refetch succeeding.

Tradeoff worth a look

Enable/disable and the per-row Run now / Test / Delete actions previously sat inline on each table row; they now live in the detail, so toggling several drains costs more clicks. That's inherent to the row pattern — a RowActionsMenu in the row's trailing slot would mean dropping the row-level <button> (a menu trigger nested in a button is invalid HTML). Flagging it as a deliberate choice rather than an oversight.

Type of Change

Refactor / improvement

Testing

Typecheck, bun run lint:check (19/19 packages), check:react-query, check:api-validation, and 92 tests across ee/data-drains, lib/data-drains, and settings all pass. Reviewed by five parallel audit passes (emcn design, hooks, React Query + URL state, comments/copy, and an adversarial regression pass) — findings applied. Not yet clicked through in a browser, so the visual pass is worth doing on the preview.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…attern Data drains was the last settings surface still on a Table plus a create modal. It now matches Skills and Custom Tools: - the list is SettingsResourceRow rows in a clickable button, with the source, destination, cadence, and last run on the row and a Disabled tag when a drain is paused - clicking a row opens a detail sub-view: the drain's actions (Run now / Test connection / Delete), its enabled toggle, resolved destination config, and recent run history — replacing the expanding table row - creating a drain is a fullscreen view instead of a modal, reusing the destination form registry so the two never fork - a drain's detail is deep-linkable via `data-drain-id`, pushed on open and replaced on close, matching custom-tool-id and custom-block-id The detail is read-only apart from the enabled toggle: destination credentials are never returned by the API, so changing one means recreating the drain. Alignment work the migration surfaced: - the destination registry rendered its 36 fields with ChipModalField, whose label is byte-identical to a SettingsSection header — on a page that made section titles and field labels indistinguishable. All of them, and the create view's own fields, now use SettingRow like every other page-level detail view - the shared source/destination/cadence label maps moved to labels.ts, now that three files need them - run status uses Badge with a dot rather than hand-coloured text, and byte counts use formatFileSize — the old /1024 math showed a 5 GB export as "5242880.0 KB" - copy follows the sibling surfaces: Create drain, Disabled, Run queued, Connection test passed, and "No drains found matching …" - seed the list cache on create so landing on the new drain's detail doesn't depend on the invalidation refetch succeeding

vercel · 2026-07-25T02:33:45Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jul 25, 2026 2:43am

cursor · 2026-07-25T02:33:47Z

PR Summary

Low Risk
UI-only refactor reusing existing APIs and hooks; no server or export-pipeline logic changes, with minor UX tradeoff (inline row actions moved into detail).

Overview
Data drains moves off the expandable table and create modal onto the same fullscreen list → detail → create flow used by Custom Tools and Skills.

The list is clickable SettingsResourceRow rows (source, destination, cadence, last run, disabled tag). Row selection is driven by a new data-drain-id URL param (push on open, replace on close) so individual drains are shareable; create stays local state only.

DataDrainCreate and DataDrainDetail are new full-page sub-views: create reuses DESTINATION_FORM_REGISTRY with unsaved-changes guarding; detail centralizes Run now / Test / Delete, an enabled toggle, read-only destination config, and recent runs (with placeholder-data guarding and formatFileSize / status badges).

The monolithic settings file shrinks accordingly; shared label maps live in labels.ts, and destination form fields switch from ChipModalField to SettingRow. useCreateDataDrain optimistically seeds the list cache so navigation to the new drain’s detail does not wait on refetch.

^{Reviewed by Cursor Bugbot for commit f51d0e1. Configure here.}

greptile-apps · 2026-07-25T02:35:46Z

Greptile Summary

The PR migrates Data Drains to the fullscreen settings list/detail pattern.

Adds URL-backed deep links for existing drain details and a local fullscreen creation flow.
Adds detail actions, status controls, destination configuration, and recent run history.
Reuses the destination registry with page-level settings rows and accessible names or label-control associations.
Extracts shared labels and updates React Query cache behavior after creation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains in the fixes associated with the previous accessibility threads.

Important Files Changed

Filename	Overview
apps/sim/ee/data-drains/components/data-drain-create.tsx	Adds the fullscreen drain creation flow with accessible controls, validation, and unsaved-change handling.
apps/sim/ee/data-drains/components/data-drain-detail.tsx	Adds the read-only drain detail surface, enabled toggle, actions, resolved configuration, and run history.
apps/sim/ee/data-drains/components/data-drains-settings.tsx	Replaces the table and modal with searchable resource rows, URL-selected details, and fullscreen creation.
apps/sim/ee/data-drains/destinations/registry.tsx	Migrates destination fields to SettingRow and correctly associates id-capable controls while naming ChipSelect directly.
apps/sim/ee/data-drains/hooks/data-drains.ts	Updates data-drain query and mutation behavior to support the new list/detail flow.
apps/sim/app/workspace/[workspaceId]/settings/[section]/search-params.ts	Adds URL state configuration for deep-linking to an existing data drain.
apps/sim/ee/data-drains/labels.ts	Centralizes source, destination, and cadence labels and select options.

_{Reviews (3): Last reviewed commit: "fix(data-drains): name the select fields..." | Re-trigger Greptile}

The ChipModalField -> SettingRow migration dropped the label/control association that ChipModalField generated for free, so screen readers announced the destination fields unnamed and clicking a label focused nothing. All 35 id-capable controls now carry an id with a matching htmlFor on their row; the one ChipSelect has no id prop, matching how the existing SettingRow + combobox rows render. Also pass includeBytes to formatFileSize — without it any run writing under 1 KB reported '0 Bytes'.

waleedlatif1 · 2026-07-25T02:38:54Z

@greptile

waleedlatif1 · 2026-07-25T02:38:55Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 4848202. Configure here.}

ChipSelect takes an aria-label that lands on its trigger button, so the four select fields no longer announce as unnamed buttons — the Datadog site plus the create view's source, cadence, and destination type. The visible SettingRow label stays; this only gives the trigger an accessible name, since ChipSelect exposes no id for htmlFor to point at.

waleedlatif1 · 2026-07-25T02:43:47Z

@greptile

waleedlatif1 · 2026-07-25T02:43:48Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit f51d0e1. Configure here.}

greptile-apps Bot reviewed Jul 25, 2026

View reviewed changes

Comment thread apps/sim/ee/data-drains/destinations/registry.tsx Outdated

cursor Bot reviewed Jul 25, 2026

View reviewed changes

Comment thread apps/sim/ee/data-drains/components/data-drain-detail.tsx Outdated

vercel Bot temporarily deployed to Preview July 25, 2026 02:38 Inactive

greptile-apps Bot reviewed Jul 25, 2026

View reviewed changes

Comment thread apps/sim/ee/data-drains/destinations/registry.tsx

cursor Bot reviewed Jul 25, 2026

View reviewed changes

vercel Bot temporarily deployed to Preview July 25, 2026 02:43 Inactive

cursor Bot reviewed Jul 25, 2026

View reviewed changes

waleedlatif1 merged commit e39045d into staging Jul 25, 2026
20 checks passed

waleedlatif1 deleted the data-drains-fullscreen branch July 25, 2026 02:49

This was referenced Jul 25, 2026

test(data-drains): cover the migrated detail and create surfaces #5958

Merged

v0.7.46: settings consolidation, ci fix, setup wizard #5963

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

improvement(data-drains): move drains to the fullscreen list/detail pattern#5954

improvement(data-drains): move drains to the fullscreen list/detail pattern#5954
waleedlatif1 merged 3 commits into
stagingfrom
data-drains-fullscreen

waleedlatif1 commented Jul 25, 2026

Uh oh!

vercel Bot commented Jul 25, 2026 •

edited

Loading

Uh oh!

cursor Bot commented Jul 25, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jul 25, 2026 •

edited

Loading

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Jul 25, 2026

Uh oh!

waleedlatif1 commented Jul 25, 2026

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

waleedlatif1 commented Jul 25, 2026

Uh oh!

waleedlatif1 commented Jul 25, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

waleedlatif1 commented Jul 25, 2026

Summary

Tradeoff worth a look

Type of Change

Testing

Checklist

Uh oh!

vercel Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

greptile-apps Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Jul 25, 2026

Uh oh!

waleedlatif1 commented Jul 25, 2026

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

waleedlatif1 commented Jul 25, 2026

Uh oh!

waleedlatif1 commented Jul 25, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Jul 25, 2026 •

edited

Loading

cursor Bot commented Jul 25, 2026 •

edited

Loading

greptile-apps Bot commented Jul 25, 2026 •

edited

Loading