Remove placeholder text from labeled form inputs#4641
Conversation
gumclaw
force-pushed
the
codex/remove-input-placeholders
branch
from
15938f7 to
adf112d
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| </FieldsetTitle> | ||
| <UtmFieldSelect | ||
| id={`${uid}-source`} | ||
| placeholder="newsletter" | ||
| baseOptionValues={context.utm_fields_values.sources} | ||
| value={data.utm_link.utm_source} | ||
| onChange={(value) => setData("utm_link.utm_source", value)} |
There was a problem hiding this comment.
🔴 The UtmFieldSelect component declares placeholder: string as a required prop, but this PR removes the placeholder prop from all 5 call sites (source, medium, campaign, term, and content fields). TypeScript will reject every call site with a missing required prop error. The fix is to change placeholder: string to placeholder?: string in the component interface.
Extended reasoning...
The UtmFieldSelect component in app/javascript/components/UtmLinks/UtmLinkForm.tsx defines its props interface with placeholder: string as a required (non-optional) prop — there is no '?' suffix. The component passes this prop directly to the inner Select component as placeholder={placeholder}.
This PR removes the placeholder prop from all 5 call sites in the same file:
- Line 382: source field (placeholder='newsletter' removed)
- Line 398: medium field (placeholder='email' removed)
- Line 415: campaign field (placeholder='new-course-launch' removed)
- Line 431: term field (placeholder='photo-editing' removed)
- Line 447: content field (placeholder='video-ad' removed)
Because placeholder has no '?' suffix in the interface, TypeScript treats it as required. Omitting a required prop at a call site is a hard compilation error. TypeScript will flag all five UtmFieldSelect usages with: 'Property placeholder is missing in type ... but required in type ...'.
Why existing code does not prevent it: The PR correctly updated the call sites but missed updating the component's own type definition. The interface still reads placeholder: string (not placeholder?: string).
Impact: The TypeScript build will fail. Any CI pipeline or bundler performing type-checking will error out and block deployment.
Step-by-step proof:
- Before PR: placeholder: string (required) and all 5 callers pass placeholder='...' — valid, compiles.
- After PR: placeholder: string (still required, unchanged) but all 5 callers no longer pass placeholder — TypeScript error at every call site.
- At runtime, placeholder would be undefined, passed straight to the inner Select component, resulting in no placeholder text being shown.
Fix: Change placeholder: string to placeholder?: string in the UtmFieldSelect props interface. Optionally also remove placeholder={placeholder} from the inner Select call or guard it with placeholder={placeholder ?? undefined}.
| aria-invalid={errors.has("email")} | ||
| value={state.email} | ||
| onChange={(evt) => dispatch({ type: "set-value", email: evt.target.value.toLowerCase() })} | ||
| placeholder="Your email address" | ||
| disabled={(loggedInUser && loggedInUser.email !== null) || isProcessing(state)} | ||
| onBlur={checkForEmailTypos} | ||
| /> |
There was a problem hiding this comment.
🟡 The ZipCodeInput and StateInput components in PaymentForm.tsx still pass the label text verbatim as placeholder (placeholder={label} where label='ZIP code'/'Postal', and placeholder={stateLabel} where stateLabel='State'/'Province'/'County'), which is exactly the redundant label-as-placeholder pattern this PR removes. The same pattern was correctly removed from AccountDetailsSection.tsx, CustomerDetailPage.tsx, and Invoices/New.tsx, making these two checkout components an inconsistency.
Extended reasoning...
What the bug is and how it manifests
In PaymentForm.tsx, two reusable sub-components were not updated when the PR cleaned up redundant label-as-placeholder patterns. ZipCodeInput (around line 190) has placeholder={label} where label is computed as "ZIP code" (for US/PH) or "Postal" (all other countries). StateInput (around line 165) has placeholder={stateLabel} where stateLabel is "State" (US/PH), "Province" (CA), or "County" (all others). Both values are identical to the visible label text displayed just above the input via a Label element.
The specific code path that triggers it
In ZipCodeInput: line 184 computes label and renders it inside a Label at line 186-188, then line 190 passes that same value as placeholder. In StateInput: stateLabel is computed in the switch statement (lines 147-160), rendered in the Label at line 162-164, then the free-text Input branch (for non-US, non-CA countries) at line 165 passes placeholder={stateLabel}. These free-text inputs are shown whenever the country is not US or CA for StateInput, and always for ZipCodeInput.
Why existing code does not prevent it
The PR diff shows that lines 344-349 in the SharedInputs component were the only section of PaymentForm.tsx touched (removing "Your email address" and "Full name" placeholders). The ZipCodeInput and StateInput components higher in the same file were not part of the diff, so the pattern was simply overlooked during the cleanup pass.
What the impact would be
Cosmetic only - no functional impact. Users in the checkout form will see placeholder text in the ZIP/Postal and State/Province/County fields that simply repeats the label, which (per the PR rationale) creates visual confusion between pre-filled and empty state. This is inconsistent with the corrected behavior in the account settings and invoice forms.
How to fix it
Remove the placeholder prop from both components: in ZipCodeInput remove placeholder={label}, and in StateInput free-text Input branch remove placeholder={stateLabel}. The visible Label above each input already communicates what the field is for.
Step-by-step proof
- User visits the checkout page for a product with shipping, selects a non-US/CA country (e.g. Germany).
- The StateInput component renders its free-text Input branch because the country is not US or CA.
- The Label shows "County"; the Input also shows "County" as placeholder - identical text in two places.
- Similarly for ZipCodeInput - the Label shows "Postal" and the Input placeholder also shows "Postal".
- Meanwhile, the equivalent ZIP/Postal field in AccountDetailsSection.tsx has no placeholder after this PR, creating an inconsistency between the checkout form and the account settings form.
Labels already describe what the input should contain. Placeholders create
confusion because users can't tell if a field is pre-filled or not.
Removed redundant placeholders from labeled fields across settings, checkout,
invoices, account details, workflow/UTM forms, and product editors.
Kept placeholders on:
- Search inputs (no visible label)
- Format/example hints (URLs, codes, money, postal/date patterns)
Where no visible label existed, add an aria-label to the source component
so the field has an accessible name:
- Affiliates/{Form,Onboarding}.tsx: aria-label="Commission"
- Profile/EditSections.tsx: aria-label="Name" on section header Input
Also update Capybara specs to use accessible labels instead of placeholder
text for inputs whose placeholders were removed.
Make placeholder optional in UtmFieldSelect props (was required but no
longer passed at call sites).
gumclaw
force-pushed
the
codex/remove-input-placeholders
branch
from
887aa18 to
21ebb1a
Compare
…greater_than_price (#4613)
The 'Street address' placeholder was removed from the Address input, so
fill_in 'Street address' no longer matches. Use find_field('Address',
match: :first) for the individual section and target by CSS id suffix
for the personal address field when the business section is also visible.
…/gumroad into codex/remove-input-placeholders
Remove placeholder attributes from inputs that already have visible labels, since they are redundant and can cause Capybara ambiguity in tests. Update specs to use `find_field` with `match: :first` or CSS selectors instead of matching on removed placeholder text. Replaces #4641 with a clean branch (no unrelated commits in diff).
3 participants
Labels already describe what the input should contain. Placeholders create confusion because users can't tell if a field is pre-filled or not.
Removed redundant placeholders from labeled fields across settings, checkout, invoices, account details, workflow/UTM forms, and product editors.
Kept placeholders on: