Skip to content

fix(platform): display env vars/secrets and passkeys as compact headerless tables (#1950)#2114

Open
larryro wants to merge 1 commit into
mainfrom
tale/xs7b1d2grqf080t9q03sx04xhd897jw7
Open

fix(platform): display env vars/secrets and passkeys as compact headerless tables (#1950)#2114
larryro wants to merge 1 commit into
mainfrom
tale/xs7b1d2grqf080t9q03sx04xhd897jw7

Conversation

@larryro

@larryro larryro commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Resolves #1950. Both env vars/secrets (the shared EnvVarListEditor, used by the agent/workflow env editors) and the account passkeys list now render as compact headerless tables following one shared minimal convention:

  • No header row, no skeleton, no empty state, no search/filter.
  • When there are zero items, the table is not rendered at all (the env editor keeps only its Add/Save controls; the passkey section renders nothing).
  • Deletion via a remove icon button per row, gated by a confirmation modal (DeleteDialog).

Changes

  • app/components/env/env-var-list-editor.tsx — rows moved into @tale/ui/table (Table/TableBody/TableRow/TableCell); removed the SkeletonText loading state and the empty-state text; the table renders only when localRows.length > 0. The per-row trash button now opens a DeleteDialog; a brand-new blank row is dropped immediately (no data to confirm).
  • app/features/settings/account/components/passkey-section.tsx — card list replaced with a headerless table; empty-state text removed; the ghost "Remove" text button is now a trash icon button gated by DeleteDialog.
  • messages/{en,de,fr}.json — removed the now-dead envEditor.none / twoFactor.passkeys.empty keys; added envEditor.confirmRemove{Title,Description} and twoFactor.passkeys.confirmRevoke{Title,Description} in all three base locales.

Verification

  • bunx tsc --noEmit (platform) — clean.
  • oxlint --type-aware on both changed files — 0 warnings, 0 errors.
  • oxfmt --check on changed files + locale JSON — clean.
  • i18n key parity across en/de/fr verified.

@larryro

larryro commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Desk review — #1950 (headerless tables for env vars/secrets & passkeys)

Verdict: READY TO MERGE. Clean, focused change that meets all three acceptance criteria on both surfaces. Reviewed in two rounds (breadth across correctness/robustness/elegance/tests/boundaries, then independent re-verification of every candidate blocker).

Verification performed

  • CI: gh pr checks45 pass, 5 skipping, 0 fail/pending (all Build/Lint/Format/Knip/Opengrep/Migrations/UI/Browser + 16 Playwright platform shards green).
  • i18n tests: bunx vitest --run lib/i18n/messages.test.ts23 passed (parity + usage enforce mode). en/de/fr at key parity for the envEditor and twoFactor.passkeys subtrees; no dangling refs to the removed envEditor.none / passkeys.empty; de-CH overlay carries no stale keys.
  • Lint: oxlint --type-aware on both changed files → 0 warnings, 0 errors.

What's good

  • Acceptance criteria met: both render as headerless @tale/ui/table tables; when empty nothing is rendered (env editor keeps only Add/Save, as intended; passkeys render nothing); per-row trash icon gated by DeleteDialog.
  • Shared EnvVarListEditor prop contract unchanged — agent-env-editor and workflow-env-editor callers unaffected.
  • Security: confirm dialogs and aria-labels expose only the key/passkey name, never secret values; the masked preview (••••) is never persisted (value field flips to a cleared password input on focus; untouched secrets are skipped on save via r.isSecret && !r.secretDirty && r.existingKey === key).
  • data-no-hover, w-0/w-48 cell sizing, and DeleteDialog usage all follow repo conventions; passkey early-return sits after all hooks (no hooks-order issue); revoking is reset in finally so the dialog can never get stuck.

Non-blocking notes (optional follow-ups, do not block merge)

  1. env-var-list-editor.tsxpendingRemove is an array index. If a reactive rows update lands while the confirm dialog is open and there are no local edits (dirty.current === false), the snapshot can shift indices and the index would point at a different row. Worst case is local-state-only until the user clicks Save, and the dialog preview reflects the changed row, so it's self-evident. Low probability for a per-agent/per-workflow settings editor. If you touch this later, keying the pending removal by the row's existingKey/a stable id instead of the index removes the hazard entirely.
  2. No unit/story tests were added for either component (the repo has none for them today; behavior is covered by the green Playwright suite). Worth a follow-up to add coverage for the confirm-remove / confirm-revoke flows.
  3. Token-source binding re-saves on every Save even when the slug is unchanged (idempotent; appears pre-existing). Minor.

None of the above gates the merge.

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.

Improvement: Display env vars/secrets and passkeys as compact headerless tables

1 participant