Skip to content

improvement(settings): consolidate resource UI onto shared primitives - #6202

Merged
waleedlatif1 merged 4 commits into
stagingfrom
sandbox-settings-ui-consistency
Aug 3, 2026
Merged

improvement(settings): consolidate resource UI onto shared primitives#6202
waleedlatif1 merged 4 commits into
stagingfrom
sandbox-settings-ui-consistency

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • SettingsResourceRow now owns the whole row — tile, title/subtitle tokens, padding + bleed, hover band, hit area, focus ring, and the one navigation chevron. New onClick/href (stretched overlay, so interactive trailing controls keep their clicks), navigable, iconVariant='custom', and a badge slot for decoration that must not swallow row clicks.
  • Sandboxes, MCP tools, and MCP servers now match custom tools: chevron + whole-row click instead of a ... menu. MCP tools/servers previously had ... → "Details" as the only way into their detail page.
  • Delete moved to the detail header as a destructive chip behind a ChipConfirmModal. Sandboxes previously deleted on a single click with no confirmation.
  • Rows across skills, integrations, api-keys, copilot, byok, access-control, data-retention, custom-blocks, data-drains, passwords and recently-deleted collapsed onto the same primitive.
  • New SettingsField; SettingsEmptyState gained tone='error'; added RESOURCE_LIST_STACK/RESOURCE_LIST_GRID, RESOURCE_TILE_PLAIN, and a shared MemberAvatar. Deleted DetailIconTile (byte-identical to ResourceTile). CredentialDetailHeading, DetailSection, and IntegrationSection are now thin aliases over the shared primitives.
  • Standardized on the emcn ArrowRight — it is a visibly different glyph from lucide's (different viewBox and stroke width), and both were in use on adjacent settings lists.
  • Fixed along the way: text-xs is 11px in this repo (not 12px), text-[14px]text-sm (different line-height), the BYOK skeleton didn't match the row it stands in for, BYOK's list ran at 4× the standard row gap, inbox rows were indented 8px against their own search field, and custom-tool detail printed the tool name twice.
  • Updated .claude/rules/sim-settings-pages.md with the resource-row contract, the chevron-vs-... rule, the delete rule, and six new checklist items; removed the section that taught hand-rolling row title/subtitle spans.

Type of Change

  • Improvement

Testing

tsc and biome clean. 630 tests across 52 files pass in every touched area, including two test files updated for the new row DOM (verified they still fail when row content breaks). Not yet exercised in a browser.

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)

Sandboxes, MCP tools, and MCP servers each modeled their list rows and
detail headers differently. Collapse them — and the surfaces they drifted
from — onto one set of primitives.

SettingsResourceRow now owns the row entirely: tile, title/subtitle tokens,
padding and bleed, hover band, hit area, focus ring, and the one navigation
chevron. Adds onClick/href (a stretched overlay, so interactive trailing
controls keep their clicks), navigable, iconVariant='custom', and a badge
slot for decoration that must not swallow row clicks.

Rows that open a detail page get a chevron and a whole-row click; flat
records keep the "..." menu. Delete moves to the detail header as a
destructive chip behind a confirm modal — sandboxes previously deleted with
no confirmation at all.

Also folds in SettingsField (new), SettingsEmptyState tone='error',
RESOURCE_LIST_STACK/GRID, RESOURCE_TILE_PLAIN, and a shared MemberAvatar;
deletes DetailIconTile (byte-identical to ResourceTile); and standardizes
on the emcn ArrowRight, which is a visibly different glyph from lucide's.
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 3, 2026 6:19am

Request Review

@cursor

cursor Bot commented Aug 3, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Large cross-surface UI refactor touching navigation, delete flows, and accessibility on many settings pages; the workflow MCP server detail view may have a JSX syntax error that would break that route at build/runtime.

Overview
This PR centralizes settings and integrations list UI on SettingsResourceRow, SettingsSection, SettingsField, and shared list/grid tokens so rows, sections, and detail fields no longer diverge across pages.

SettingsResourceRow now owns the full row: tile variants (tile / plain / custom), title/subtitle tokens, bleed/padding, hover, focus, optional href or onClick via a stretched overlay (so trailing controls stay clickable), navigable chevron, badge for click-through decoration, and RESOURCE_LIST_STACK / RESOURCE_LIST_GRID containers. Integrations, API keys, MCP, sandboxes, passwords, BYOK, custom tools, recently deleted, and related surfaces migrate off hand-rolled Link / button wrappers and duplicate arrow markup.

Navigation pattern shifts for several resources: detail pages use whole-row click + chevron instead of a ... menu with “Details”; delete moves to the detail header as a destructive action behind ChipConfirmModal (sandboxes, MCP servers, workflow MCP). SettingsEmptyState gains tone='error'; credential detail aliases (CredentialDetailHeading, DetailSection) and IntegrationSection wrap the shared primitives; DetailIconTile is removed in favor of ResourceTile / tile constants including RESOURCE_TILE_PLAIN.

Docs/skills (sim-settings-pages.md, add-settings-page) are expanded with the resource-row contract, chevron vs menu rule, and migration checklist. Note: workflow-mcp-servers.tsx appears to contain invalid JSX where modal blocks were refactored (**)canManage && (** instead of proper {canManage && ( / )} closures)—worth verifying before merge.

Reviewed by Cursor Bugbot for commit df4263b. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR consolidates resource lists and detail surfaces onto shared settings primitives while standardizing navigation, deletion confirmation, spacing, typography, and empty states.

  • Expands SettingsResourceRow to own navigation, row interaction, badges, tiles, and chevrons.
  • Moves resource deletion into confirmed detail-header actions.
  • Replaces duplicated list, section, field, tile, and avatar implementations with shared components.
  • Updates settings design-system guidance and affected tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/settings/components/settings-resource-row/settings-resource-row.tsx Expands the shared resource-row primitive to provide consistent layout, navigation overlays, accessibility, trailing controls, and visual treatment.
apps/sim/app/workspace/[workspaceId]/settings/components/sandboxes/sandboxes.tsx Migrates sandbox rows and detail deletion to the shared interaction model; the identity guard correctly clears confirmation state when selection changes.
apps/sim/app/workspace/[workspaceId]/settings/components/sandboxes/components/sandbox-editor.tsx Adds confirmed deletion to the sandbox detail header and displays the selected sandbox in confirmation copy.
apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx Converts MCP resources from menu-only detail access to whole-row navigation using the shared primitive.
apps/sim/app/workspace/[workspaceId]/components/credential-detail/components/credential-detail-heading.tsx Reuses the static shared resource row for consistent credential-detail heading layout and typography.
.claude/rules/sim-settings-pages.md Documents the consolidated resource-row, deletion, field, section, tile, and list-container contracts.

Reviews (3): Last reviewed commit: "fix(settings): wire flush, drop the lone..." | Re-trigger Greptile

…seeding

Follow-up from review of the consolidation.

The sandbox delete confirmation was boolean-only state. Browser Back unmounts
the modal without closing it, so opening another sandbox re-opened it already
confirmed — against the wrong sandbox. Reset it where the draft guard already
handles the same history hazard.

RESOURCE_LIST_GRID kept an 8px column gap after the bleed moved onto the row,
so neighbouring cards overlapped by exactly the gutter and the right-hand card's
stretched hit area won it: clicking between two cards opened the wrong one.
Budget the gap for the bleed instead.

Also: restore the `group` the template-icon hover outlines depend on; add a
`flush` row for headings and overflow boxes; seed MemberAvatar identically on
every surface; keep the MCP transport label visible in all row states; give the
Delete chip a stable id so it doesn't remount mid-delete; and stop an empty
subtitle rendering a phantom line.

Docs: widen the rule's path globs to the surfaces it now governs, drop the
`text-[14px]` example that contradicted the token rule, and add a Mode C for
migrating rows onto the primitive.
Consolidating the Teammates/Organization avatar onto the credential member
row's was a redesign, not a deduplication. The two encode different things:
the roster is a dense list keyed on email where the avatar is a 14px neutral
marker, while a member management row carries a name, an email, and a role
control and earns a 36px hashed avatar.

Merging them made those rows ~70% taller, gave every workspace in a permission
group a colour-hashed "avatar" seeded on its name, and cut the Add Members
picker from ~7 visible rows to ~5 inside its fixed-height box.

Restores both avatars and the containers that owned their bleed, and records
in the rule why they stay separate. Keeps only the unrelated fix in that area:
the picker row was the one settings row rounded at `sm`.
Final review round. Two fixes the previous commit claimed but did not land:
`CredentialDetailHeading` never passed `flush`, so every credential detail
heading wore list-row padding, and the empty-subtitle guard was never applied.
Both were scripted replacements that silently no-op'd.

RESOURCE_LIST_GRID also dropped to one column 32px earlier than the grid it
replaced — `auto-fit` measures tracks, not margin boxes, so widening the gap
for the rows' bleed moved the breakpoint. Track minimum now budgets for it.

Removes the BYOK skeleton rather than maintaining a second copy of the row: it
was the only skeleton in settings, and it had already desynced from the row it
imitates. Its peers render nothing while loading.

Also: unify the glyph-tile treatment across MCP/sandboxes/workflow-MCP with
custom tools; move decoration out of `trailing` in verified-domains and
recently-deleted; convert the last hand-rolled row and empty states in
workflow-MCP, api-keys, copilot and group-detail; give copilot's delete the
same `...` affordance as api-keys; announce the row description via
aria-describedby, which the stretched overlay had silenced; and let
SettingsField render its own value so callers stop restating type tokens.

Docs: correct claims that predate this PR — `aside` does not exist, the
navigation source of truth is under components/, beforeunload mounts in the
layouts, `getSettingsSectionMeta` takes two args — and stop asserting a
literal-pixel grep returns zero when display type legitimately uses it.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

All three review findings are fixed and the threads are resolved:

  • Template icon hover outline (Bugbot) — SettingsResourceRow emits group again; group-hover:outline tracks the row background. f381712ea2
  • Grid row hit areas overlap (Bugbot) — the column gap now budgets for the rows' -mx-2, and the track minimum was lowered so the two-column breakpoint doesn't move. f381712ea2 + df4263bbc0
  • Delete confirmation loses its target (Greptile, P1) — the confirm state resets in the existing identity guard, and the copy now names the sandbox. f381712ea2 + df4263bbc0

Three commits landed since the last review, including a revert of the Teammates/Organization avatar change (that was a redesign, not a deduplication) and a round of fixes from a further audit: a dead flush prop that was documented but never wired, the row description no longer being announced by screen readers behind the stretched overlay, the BYOK loading skeleton removed rather than maintained as a second copy of the row, and several rows/empty states in touched files that were still hand-rolled.

CI is green on df4263bbc0. Please re-review the current head.

@greptile review
@cursor review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Greptile has re-reviewed df4263bbc0 and its check is green. Bugbot last ran on the first commit 80a01f027b; three commits have landed since, including fixes for both issues it raised.

bugbot run

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 df4263b. Configure here.

@waleedlatif1
waleedlatif1 merged commit 786c854 into staging Aug 3, 2026
27 checks passed
@waleedlatif1
waleedlatif1 deleted the sandbox-settings-ui-consistency branch August 3, 2026 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant