refactor(forms)!: stop auto-colouring fields on native :user-invalid/:user-valid - #603
Conversation
…:user-valid optional/forms.css flipped --sf-field-border-color to --sf-color-danger / --sf-color-success automatically whenever the browser's own constraint validation (:user-invalid / :user-valid) fired. A required or pattern-constrained field could render "broken" (red border) as soon as it was touched, before the user submitted anything or the app ran its own validation — the framework silently overriding an element's appearance based on browser-internal state the consumer never opted into. This is the same category of surprise as the .sf-card/.sf-btn auto-shrink rule removed previously, so it is removed too. - Delete the input/select/textarea :user-invalid / :user-valid rules (optional/forms.css). The token itself, its resting/focus/autofill consumers, and the explicit .sf-is-invalid / .sf-is-valid / .sf-is-warning / .sf-is-info / .sf-is-danger state classes (core/states.css) are unchanged — they remain the one way to colour a field's validation state. - No token-registry/docs/demos regeneration needed: --sf-field-border-color was never declared as a :root token (pure var()-fallback hook), so its removal from the automatic trigger doesn't change the public token surface. - Update stale test comments referencing the removed mechanism (tests/forms.spec.js). No new Playwright assertion was added for the removal itself: :user-invalid does not reliably match via synthetic focus/blur in headless automation (verified — it didn't even match before this change), so a test asserting its absence would be flaky by construction. The explicit path already has solid dedicated coverage in tests/states-full.spec.js. - Document the break in docs/migration.md and CHANGELOG.md. Verified: full node --test unit suite (109/109) and all 10 CSS gates pass with no registry drift, confirming the token surface is untouched. BREAKING CHANGE: form fields no longer auto-colour their border on native :user-invalid / :user-valid. Add .sf-is-invalid / .sf-is-valid explicitly when your code determines a field's validation state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnwqYHgHNAWPbsUJWUeY1w
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe forms stylesheet no longer auto-colours fields from native ChangesForm validation styling
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR removes the
Confidence Score: 5/5Safe to merge — a clean, targeted deletion of 13 CSS lines with no effect on the remaining token surface, consumers, or test logic. The CSS removal is minimal and surgical: only the two :user-invalid/:user-valid rule blocks are gone, every other use of --sf-field-border-color (resting border, focus, autofill) is untouched. A repo-wide search confirms no stale references to the removed pseudo-classes remain outside the new migration guide and changelog entry. The llm-guide description for --sf-field-border-color remains accurate because the explicit state classes still set it. Test comments are updated to match; no test logic is changed. Breaking-change documentation is complete and accurate. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User interacts with field] --> B{Validation trigger}
B -->|BEFORE this PR| C[Browser fires :user-invalid / :user-valid]
C --> D[forms.css auto-sets --sf-field-border-color to danger / success]
D --> E[Field border changes immediately on first touch]
B -->|AFTER this PR| F[Browser fires :user-invalid / :user-valid]
F --> G[No CSS rule matches — border unchanged]
B --> H[App adds .sf-is-invalid / .sf-is-valid]
H --> I[core/states.css sets --sf-field-border-color]
I --> J[Field border changes on app's own terms]
%%{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[User interacts with field] --> B{Validation trigger}
B -->|BEFORE this PR| C[Browser fires :user-invalid / :user-valid]
C --> D[forms.css auto-sets --sf-field-border-color to danger / success]
D --> E[Field border changes immediately on first touch]
B -->|AFTER this PR| F[Browser fires :user-invalid / :user-valid]
F --> G[No CSS rule matches — border unchanged]
B --> H[App adds .sf-is-invalid / .sf-is-valid]
H --> I[core/states.css sets --sf-field-border-color]
I --> J[Field border changes on app's own terms]
Reviews (1): Last reviewed commit: "refactor(forms)!: stop auto-colouring fi..." | Re-trigger Greptile |
Why
Follow-up to #602 (removing the
.sf-card/.sf-btnauto-shrink). An audit of the whole framework for "the component silently decides something for you" patterns turned up one more:optional/forms.cssflipped--sf-field-border-colorto--sf-color-danger/--sf-color-successautomatically whenever the browser's own constraint validation (:user-invalid/:user-valid) fired.That means a
requiredorpattern-constrained field could render "broken" (red border) as soon as it was touched — before the user submitted anything, and before the app ran its own validation logic. Same category of surprise as the card/button rule: the framework overriding an element's appearance based on browser-internal state the consumer never opted into.What
input/select/textarea:user-invalid/:user-validrules inoptional/forms.css(13 lines).--sf-field-border-colortoken itself and all its consumers (resting border, focus, autofill), and the explicit state classes incore/states.css—.sf-is-invalid/.sf-is-error,.sf-is-valid/.sf-is-success,.sf-is-warning,.sf-is-info,.sf-is-danger. These remain the one way to colour a field's validation state — add the class when your code decides the field is invalid.--sf-field-border-colorwas never declared as a:roottoken (purevar()-fallback hook, only ever set by the removed rule and by the state classes), so removing the automatic trigger doesn't change the public token surface at all. Confirmed: 755 tokens / registry+0/-0before and after.tests/forms.spec.jsthat referenced the removed mechanism.docs/migration.mdandCHANGELOG.md.On testing this
I did not add a Playwright assertion asserting the removal directly:
:user-invaliddoesn't reliably match via syntheticfocus()/blur()in headless automation (verified in Chromium — it didn't even match before this change, so a test built on that trigger would be flaky by construction, not a real regression guard). The explicit path already has solid, non-flaky coverage intests/states-full.spec.js(.sf-is-invalid/.sf-is-valid/etc. → correct--sf-field-border-color), which is untouched by this PR.Verification
Full node
--testunit suite (109/109) and all 10 CSS gates pass, confirming no token-surface drift.Notes
!commit +BREAKING CHANGE:footer);docs/migration.mdupdated so the migration-docs CI gate passes.main(includes refactor(components)!: drop the .sf-card auto-shrink of nested button labels #602) — fresh PR, isolated 4-file diff.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
.sf-is-invalidand.sf-is-validto control field styling.