Skip to content

Refine the connect modal's API-key credential UX#1192

Closed
RhysSullivan wants to merge 1 commit into
lock-detected-auth-tabsfrom
connect-modal-ux
Closed

Refine the connect modal's API-key credential UX#1192
RhysSullivan wants to merge 1 commit into
lock-detected-auth-tabsfrom
connect-modal-ux

Conversation

@RhysSullivan

@RhysSullivan RhysSullivan commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

Refines how a user reads and enters an API-key credential in the connect modal. Three related changes:

  • Merged credential field. The placement's lead + prefix (e.g. Authorization: Bearer ) is a fixed, non-editable addon inside the credential input (its own muted segment + divider), so the field reads as the header value being built and you type only the token. The separate Authorization: Bearer •••••• preview line is dropped. autoComplete="off" plus password-manager ignore attributes stop the browser from offering to generate a password here; the app's own 1Password picker still covers filling an existing secret.
  • Attached tabs. The auth-method tabs (API key / OAuth2 / +) are joined to the panel as one card: a full-width, rounded-top header with a hairline divider into the content, instead of a pill bar floating above a separate card.
  • Prefix trailing-space warning (amber). A non-empty prefix with no trailing space is sent joined to the value (Bearer + token → Bearertoken), almost always a mistake. An amber warning surfaces in the placement editor; it's advisory, not blocking, and clears once a space is added. (The design system holds color to destructive red, so this amber is a deliberate, scoped exception.)

Stacked on #1188 (base lock-detected-auth-tabs) because the previews here rely on the PlacementLine whitespace fix from that PR.

Recording

Connect modal → merged credential field → Add authentication method (presets + placement editor) → a bare prefix warns, and restoring the space clears it.

Connect modal · API key credential UX

Tests

New selfhost browser scenario e2e/selfhost/connect-modal-credential-ux.test.ts covers the merged affix field, the add-method editor's presets, and the prefix warning appearing then clearing.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 28, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
executor-marketing 2692f42 Commit Preview URL

Branch Preview URL
Jun 28 2026, 11:19 PM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 28, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
executor-cloud 2692f42 Jun 28 2026, 11:19 PM

@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Cloudflare preview

Console https://executor-preview-pr-1192.executor-e2e.workers.dev
MCP https://executor-preview-pr-1192.executor-e2e.workers.dev/mcp
Deployed commit 2692f42

Sign-in is Cloudflare Access (one-time PIN to an allowed email). The preview has its own database and encryption key; it is destroyed when this PR closes.

@pkg-pr-new

pkg-pr-new Bot commented Jun 28, 2026

Copy link
Copy Markdown

Open in StackBlitz

@executor-js/cli

npm i https://pkg.pr.new/@executor-js/cli@1192

@executor-js/config

npm i https://pkg.pr.new/@executor-js/config@1192

@executor-js/execution

npm i https://pkg.pr.new/@executor-js/execution@1192

@executor-js/sdk

npm i https://pkg.pr.new/@executor-js/sdk@1192

@executor-js/codemode-core

npm i https://pkg.pr.new/@executor-js/codemode-core@1192

@executor-js/runtime-quickjs

npm i https://pkg.pr.new/@executor-js/runtime-quickjs@1192

@executor-js/plugin-file-secrets

npm i https://pkg.pr.new/@executor-js/plugin-file-secrets@1192

@executor-js/plugin-graphql

npm i https://pkg.pr.new/@executor-js/plugin-graphql@1192

@executor-js/plugin-keychain

npm i https://pkg.pr.new/@executor-js/plugin-keychain@1192

@executor-js/plugin-mcp

npm i https://pkg.pr.new/@executor-js/plugin-mcp@1192

@executor-js/plugin-onepassword

npm i https://pkg.pr.new/@executor-js/plugin-onepassword@1192

@executor-js/plugin-openapi

npm i https://pkg.pr.new/@executor-js/plugin-openapi@1192

executor

npm i https://pkg.pr.new/executor@1192

commit: 8d43d83

@greptile-apps

greptile-apps Bot commented Jun 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR refines the connect modal's API-key credential UX: the placement prefix is merged into the credential field as a non-editable affix, the auth-method tabs are visually attached to the panel card, and the placement editor gains preset buttons and an amber warning for bare prefixes (no trailing space).

  • Merged credential fieldsingleCredentialAffix computes the placement's lead + prefix and renders it as a fixed, non-selectable inline affix; the separate preview line is dropped.
  • Attached tabsgap-3gap-0 and complementary rounded-* adjustments join the TabsList and TabsContent into one visual card.
  • Placement-editor improvements — three preset buttons (Bearer header / API key header / API key query) and a barePrefix warning are added to placement-editor.tsx.

Confidence Score: 4/5

Mostly safe to merge; one code path silently hides a placement from the user when a single-credential method sends to more than one non-env carrier.

The singleCredentialAffix calculation uses find(), which returns the first non-env placement and then suppresses the old placement-preview section for every method where an affix is non-null. A method configured with two non-env placements sharing one credential (e.g., Authorization header + query fallback) would present only the first placement in the field affix while the second placement disappears from the UI entirely. The three-file change is otherwise well-structured and the prefix-warning logic is correct.

packages/react/src/components/add-account-modal.tsx — specifically the singleCredentialAffix memo and the placement-preview guard at line 1517.

Important Files Changed

Filename Overview
packages/react/src/components/add-account-modal.tsx Adds affix prop to PasteCredentialInputs, computes singleCredentialAffix for merged credential field, and attaches auth-method tabs to panel as one card. singleCredentialAffix uses find() (first non-env placement) and then suppresses the old placement preview; methods with 2+ non-env placements sharing one credential variable silently lose the extra placement from the UI.
packages/react/src/components/placement-editor.tsx Adds placement presets (Bearer header / API key header / API key query) and an amber warning when any prefix lacks a trailing space. Logic is correct — prefix is typed as string so .length / .endsWith are safe.
e2e/selfhost/connect-modal-credential-ux.test.ts New selfhost browser scenario covering merged affix, placement presets, and prefix warning. Steps are clear and readable. The test does not include an assertion that the separate preview line is absent (per a flagged existing thread), and the needs capability object is empty ({}) despite using Browser.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[method + singleInput + !isEnvMethod] --> B{singleCredentialAffix?}
    B -- "non-null (1 non-env placement)" --> C[Show affix inside credential input\nHide separate placement preview]
    B -- "null (0 or multi non-env placements)" --> D[Show placement preview lines\nShow plain credential input]
    C --> E[PasteCredentialInputs renders\naffix span + raw input]
    D --> F[PasteCredentialInputs renders\nstandard Input]
    G[PlacementEditor] --> H{barePrefix?}
    H -- yes --> I[Amber warning: 'Prefix has no trailing space']
    H -- no --> J[No warning]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[method + singleInput + !isEnvMethod] --> B{singleCredentialAffix?}
    B -- "non-null (1 non-env placement)" --> C[Show affix inside credential input\nHide separate placement preview]
    B -- "null (0 or multi non-env placements)" --> D[Show placement preview lines\nShow plain credential input]
    C --> E[PasteCredentialInputs renders\naffix span + raw input]
    D --> F[PasteCredentialInputs renders\nstandard Input]
    G[PlacementEditor] --> H{barePrefix?}
    H -- yes --> I[Amber warning: 'Prefix has no trailing space']
    H -- no --> J[No warning]
Loading

Reviews (2): Last reviewed commit: "Refine the connect modal's API-key crede..." | Re-trigger Greptile

Comment thread e2e/selfhost/connect-modal-credential-ux.test.ts
Comment thread packages/react/src/components/add-account-modal.tsx
- Merged credential field: the placement's lead + prefix is a fixed,
  non-editable addon inside the field, so it reads as the header value being
  built ("Authorization: Bearer <token>") and the separate preview line is
  dropped. autoComplete/ignore attrs stop the browser and password managers
  from offering to GENERATE a password here; the app's own 1Password picker
  still covers filling an existing secret.
- Attached tabs: the auth-method tabs join the panel as one card (full-width
  rounded-top header, hairline divider) instead of a pill bar floating above a
  separate card.
- Prefix trailing-space warning (amber): a non-empty prefix with no trailing
  space is sent joined to the value ("Bearer" + token -> "Bearertoken").
  Advisory, not blocking.
@RhysSullivan RhysSullivan deleted the branch lock-detected-auth-tabs June 28, 2026 23:18
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