Refine the connect modal's API-key credential UX#1195
Conversation
- 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.
Cloudflare previewTorn down — the PR is closed. |
Greptile SummaryThis PR refines the connect modal's API-key credential UX with three coordinated changes: a merged credential field that renders the placement's lead and prefix as a non-editable affix inside the input, tab-list styling that attaches the method tabs flush to the content panel, and an amber advisory warning in the placement editor when a prefix has no trailing space.
Confidence Score: 4/5Safe to merge for the common single-placement case; the edge cases noted are worth addressing before shipping to users who configure multi-placement methods. The core affix UX and warning logic are correct for the typical bearer/API-key method. Two edge cases stand out: the raw input in the affix branch has no accessible label (screen readers get nothing useful), and when a method carries more than one non-env placement the second placement silently disappears from the UI because both the preview section and the affix only cover the first. packages/react/src/components/add-account-modal.tsx — the affix input branch and the placement-preview gating condition. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[method resolved] --> B{singleInput AND NOT isEnvMethod?}
B -- No --> C[Plain credential input, no affix]
B -- Yes --> D[Find first non-env placement]
D --> E{Placement found?}
E -- No --> C
E -- Yes --> F[Build lead: header = 'Name: ', query = '?name=']
F --> G[Append prefix: affix = lead + prefix]
G --> H[singleCredentialAffix is truthy]
H --> I[Render merged field: fixed affix span + raw input]
H --> J[Hide placement-preview section]
J --> K{Multiple non-env placements?}
K -- Yes --> L[Only first affix shown - others invisible]
K -- No --> M[Single placement shown correctly]
N[PlacementEditor] --> O{barePrefix? prefix non-empty and no trailing space}
O -- Yes --> P[Amber warning banner]
O -- No --> Q[No warning]
%%{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 resolved] --> B{singleInput AND NOT isEnvMethod?}
B -- No --> C[Plain credential input, no affix]
B -- Yes --> D[Find first non-env placement]
D --> E{Placement found?}
E -- No --> C
E -- Yes --> F[Build lead: header = 'Name: ', query = '?name=']
F --> G[Append prefix: affix = lead + prefix]
G --> H[singleCredentialAffix is truthy]
H --> I[Render merged field: fixed affix span + raw input]
H --> J[Hide placement-preview section]
J --> K{Multiple non-env placements?}
K -- Yes --> L[Only first affix shown - others invisible]
K -- No --> M[Single placement shown correctly]
N[PlacementEditor] --> O{barePrefix? prefix non-empty and no trailing space}
O -- Yes --> P[Amber warning banner]
O -- No --> Q[No warning]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {/* oxlint-disable-next-line react/forbid-elements */} | ||
| <input | ||
| type="password" | ||
| autoComplete="off" | ||
| data-1p-ignore | ||
| data-lpignore="true" | ||
| data-bwignore | ||
| data-form-type="other" | ||
| placeholder="token" |
There was a problem hiding this comment.
<input> in the affix branch has no programmatic label. When affix is truthy, no <Label> is rendered and there is no id to associate one with, so screen readers will announce this field with only its type ("password edit") and no description. Adding an aria-label directly on the element covers the gap without altering the visual layout.
| {/* oxlint-disable-next-line react/forbid-elements */} | |
| <input | |
| type="password" | |
| autoComplete="off" | |
| data-1p-ignore | |
| data-lpignore="true" | |
| data-bwignore | |
| data-form-type="other" | |
| placeholder="token" | |
| {/* oxlint-disable-next-line react/forbid-elements */} | |
| <input | |
| aria-label={`${input.label} value`} | |
| type="password" | |
| autoComplete="off" | |
| data-1p-ignore | |
| data-lpignore="true" | |
| data-bwignore | |
| data-form-type="other" | |
| placeholder="token" |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Refines how a user reads and enters an API-key credential in the connect modal. Three related changes:
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 separateAuthorization: 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.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.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.)Recording
Connect modal → merged credential field → Add authentication method (presets + placement editor) → a bare prefix warns, and restoring the space clears it.
Tests
New selfhost browser scenario
e2e/selfhost/connect-modal-credential-ux.test.tscovers the merged affix field, the add-method editor's presets, and the prefix warning appearing then clearing.