Repository navigation
fix(forms): stop placeholders reading as values a blank field doesn't submit - #1887
Merged
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request improves user experience and form validation by updating several input placeholders for clarity, adding accessibility labels, and ensuring that empty or whitespace-only OAuth scope inputs default to a standard fallback value. Additionally, a new test suite has been added to verify the OAuth scope behavior under various conditions. There are no review comments, and the changes look correct and well-tested.
… submit
A placeholder that shows a real-looking value reads as already filled in, so
users either leave the field blank expecting that value or retype it by hand.
Each one now either is what a blank field submits, or reads as an example:
- OAuth Scope: a blank field now submits the default its placeholder shows.
Central manager stores the scope verbatim and it overrides the provider
preset, so a blank Scope signed in with an empty scope.
- Install Command (import + redeploy): the "npm install" placeholder said
what to type, but a blank field already gets Harper's automatic install,
and typing "npm install" opts out of it. The placeholder now says so.
- Okta domain, load balancer host name, HNSW index type and the API
explorer bearer token read as examples ("e.g. ...").
Also gives the OAuth provider card's icon-only edit button an accessible
name ("Edit provider").
Closes #1294
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ing it on submit Text inputs whose zod schema caps a length now carry a matching `maxLength`, read from the schema itself (`Schema.shape.<field>.maxLength`) so the attribute and the validation cannot come apart. That covers the organization role name, the add/invite user email, the instance role name, sign-up first/last name and email, profile first/last name, the new application name, the add/rename file names, and the import modal's database and table names. Password inputs are deliberately left without `maxLength`: a clamped paste would save a password in the password manager that never worked, so they keep showing the validation error instead. The profile form's name limits (50/50) disagreed with central manager's `validateUpdateUser` (first name 40, last name 80), so a 41-50 character first name passed the form and was refused by the server, and a 51-80 character last name that sign-up accepted could never be saved again. They now match the server and the sign-up form. Closes #1290 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…was cut A hard maxLength stops typing at the limit, but it also truncates a longer paste without a word, so a password-manager or clipboard value can land cut short with nothing on screen to say so. Every field #1888 limits now says so under the input, in amber rather than the red of a validation error, since the value is valid: - "Maximum N characters reached." once the value reaches the limit, cleared again once it drops back under. A value that starts over the limit, such as a Profile name saved under the old 50-character cap, says how long it is. - "... Pasted text was P characters; only the first K fit." (or "none of it fit") when a paste was cut, measured once the browser has inserted it: from the selection where the input exposes one, and, where type="email" hides it, by reading the new value as the old one with the paste spliced in, so text already in the field never counts as pasted. It counts what actually landed, which is one short of the room when the browser won't split a surrogate pair, and it stays until the value changes. A schema without .max() gives zod's maxLength as null, which leaves the field alone rather than showing "Maximum null characters". The hint is an always-rendered aria-live="polite" region tied to the input by aria-describedby (FormControl now appends a caller's id rather than dropping it), and shows only the paste message after a cut paste rather than announcing the limit first. Password fields get neither a maxLength nor a hint. LengthLimitedInput wraps FormControl + Input for the plain fields; AuthInput uses the same useLengthLimit hook, so sign-up's names and email get it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d fields hit their limit
A cut email address can still be a valid one: an 81-character
...@example.com pasted into the organization invite or sign-up became
...@example.co, passed z.email(), and the invite would have gone to that
mailbox. So the two email fields now work like the password fields: no
maxLength and no length hint, and the schema's .max(80) refuses an over-long
address on submit ("Email cannot be longer than 80 characters.").
That leaves no capped field with type="email", the one input type that hides
its selection, so measurePasteCut keeps only the selection-known path. The
hook makes no paste claim when the selection is hidden.
NewOrgForm (name, subdomain), ClusterName, ClusterAbbreviatedName and
CreateNewTableModal (table name, primary key, database name) already had a
maxLength that cut silently. They now go through LengthLimitedInput, so they
say when the limit is reached and when a paste was cut, like the rest.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reset Password refused anything over 50 characters, while sign-up and Profile set no limit at all. None of those matched central manager: its only password cap is `VALIDATION_CONSTRAINTS.USER.PASSWORD_MAX_LENGTH` (256), applied by `validateAddUser` to sign-up, while `validateResetPassword` and `validateUpdateUser` take any length. So a password manager's 64-character password worked for sign-up and Profile but could not be reset to, and a sign-up password over 256 passed the form only to be refused by the server. All three forms now cap the password at one shared `ACCOUNT_PASSWORD_MAX_LENGTH` (256) with the same message. Password inputs still have no `maxLength`, so an over-long paste is shown in full with the error rather than silently clamped. Closes #1883 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dawsontoth
force-pushed
the
claude/1294-placeholder-usability
branch
from
October 12, 2026 23:23
53c9df4 to
413bf83
Compare
dawsontoth
marked this pull request as ready for review
October 12, 2026 23:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Folded in
This PR now also carries the form length-limit stack, rebased in a straight line on stage, so one Rebase and merge lands all three:
Decided in review: database, table, file and application names keep their length cap, which says so when it cuts a paste. Only email and password fields have no cap.
Each of those PRs keeps its own verification and cross-model review record. On the combined tree, the full suite passes (505 files, 4,734 tests), and
tsc -b, oxlint anddprint checkall exit 0.⊙ Problem
Some Fabric placeholders show a real-looking value that a blank field does not submit (#1294). Users read the field as already filled and leave it blank, or retype the grey text by hand. The worst case is OAuth Scope: a blank Scope saved an empty string, and that empty string replaces the provider's preset scope, so sign-in ran with no scope at all.
💡 Solution
I went through all ~70 placeholders in
srcand sorted each one with a single rule. A placeholder that shows a value must either be what a blank field submits, or read as an example. Instructions ("Search…", "Choose Role") and placeholders that already match what a blank field submits are left alone (see Left unchanged below).openid email profile, but a blank field sent''openid email profilecompany.okta.come.g. company.okta.comexample.your-company.come.g. cluster.your-company.comnpm installLeave blank to install dependencies automaticallynpm installis not the same thing: it opts out of the automatic install. Evidence belowHNSWe.g. HNSWeyJhbGciOi…e.g. eyJhbGciOi…Evidence for the behavioral claims (traced, not assumed):
''for a blank Scope. Central manager passes the stored value to@harperfast/oauthunchanged. That library'sbuildProviderConfigspreads it over the preset:config.ts:79-82@ v1.6.0, the version central manager pins. v2.9.0 still keeps an explicit''(config.ts:548-552). The authorize request then sendsscope: this.config.scope || ''(OAuthProvider.ts:76@ v1.6.0).Application.ts:4402). Otherwise it uses the package manager frompackage.jsondevEngines.packageManager(:4457), and falls back to npm with--omit=dev(:4540). A redeploy with a blank command writes noinstallblock (operations.js:793). Harper 5.0.0 and 4.7 use the same order: custom command, thendevEngines, then npm.CUSTOM_INDEXESholds onlyHNSW(customIndexes.ts:4), and a blank type returns early as a standard index (databases.ts:3482). Distance left blank is cosine (HierarchicalNavigableSmallWorld.ts:543-548), so itscosineplaceholder stays.Left unchanged
idand Databasedata, Import Data's Databasedata, the schema editor's Databasedata, table/endpoint name overrides and Distancecosine, and the new application's Name.Choose …selects,Your password,Leave blank to keep existing,Okta Client ID/Secret,Optional,Page.e.g. us-east-1,e.g. 100OFF,e.g. example.com, *.example.com,e.g. https, operations, wss,e.g. 280ms, medium distribution.you@company.comon the auth pages reads as a template. The secret editor's********deliberately signals a hidden existing value, next to Reveal. The cluster instance protocol select'shttps://placeholder never shows, becausesecureis always set. In the admin-only notification composer, the link sample lists two alternatives joined by "or", and a blank message is rejected when you submit.Skipped (would conflict)
https://example.com/data.csv→e.g. https://example.com/data.csv. I skipped it because open PR feat(status): list Harper jobs on a Jobs tab, and follow a CSV import's job in a toast #1880 addsImportDataModal.progress.test.tsx, which queriesgetByPlaceholderText('https://example.com/data.csv'). That test would break when the two merge. It is a one-line follow-up once feat(status): list Harper jobs on a Jobs tab, and follow a CSV import's job in a toast #1880 lands.🔧 Changes
src/features/organization/settings/index.tsx:DEFAULT_OAUTH_SCOPEconstant. It is used as the add form's default value and in both Scope placeholders (edit, add).e.g.(edit, add).aria-label="Edit provider", which the new test also uses to find it.e.g.:ClusterFQDN.tsxFieldRow.tsxSettingsPanel.tsx(dprint re-wrapped this line)RedeployApplicationModal.tsxandImportInstructions.tsx.✅ Verification
src/features/organization/settings/index.test.tsxrenders the real page against a mockedapiClient. It checks three things: that a blank Scope on add sends exactly the placeholder's value, that an edit of a provider with no stored scope sends the default, and that a typed scope is kept. Mutation-checked: with both fallbacks reverted, the first two tests fail (expected ' ' / '' to be 'openid email profile').stagequeries those placeholders, and none of the open PRs that touch these files changes or queries them (checked feat(deploy): follow a rolling deploy's restart job, and show canary certification #1868, Show a failed app import or redeploy once, and keep a failed import beside its form #1871, feat(deploy): stage a release now and activate it later (Harper 5.3+) #1876, fix(sidebars): resize every sidebar with one pointer hook that never sticks mid-drag #1805 and fix(auth): finish sign-out when a storage write throws #1813). feat(status): list Harper jobs on a Jobs tab, and follow a CSV import's job in a toast #1880 does query the CSV URL placeholder, which is why that row was skipped.npx vitest runon all touched directories, 19 files / 221 tests, exit 0.npx tsc -bexit 0,npx oxlintexit 0,npx dprint checkexit 0. The pre-commit hook ran the full suite (396 files, 3,666 passed / 11 skipped), oxlint, dprint and commitlint, all green.git fetch. The Harper domain adjudicator failed on an expired Claude OAuth session, so the outside findings were triaged by hand:@/features/organization/settings, and instead of a comment explaining an unnamed-button selector, the edit button has an accessible name. I also renamed the add test, since gemini pointed out it clears Scope to whitespace rather than leaving it untouched.transform. Both schemas are module-private with one consumer each. A transform would splituseForm's input and output types to protect a hypothetical second consumer.FormDescriptionwould also collide with open PR feat(deploy): stage a release now and activate it later (Harper 5.3+) #1876, which adds that same import to both files.DEFAULT_OAUTH_SCOPE. It states a constraint the code can't show: a blank scope is stored as-is and overrides the provider preset.Closes #1294
Closes #1290
Closes #1883
🤖 Generated with Claude Code
Related PRs: none found
Complexity: easy
Review-Coverage: authored=claude; ran=gemini,codex; blocked=cursor-grok(no-receipt),cursor-composer(no-receipt),domain(auth); declined=cursor-kimi,cursor-muse; rounds=1; full=1 @ 53c9df4
Review-Attention: study ~11m (critical: FieldRow.tsx; raised: degraded review) @ 53c9df4