feat(form-field): browser autofill tints the field instead of overriding its state (#DS-4096) - #1908
Open
lskramarov wants to merge 10 commits into
Open
feat(form-field): browser autofill tints the field instead of overriding its state (#DS-4096)#1908lskramarov wants to merge 10 commits into
lskramarov wants to merge 10 commits into
Conversation
Autofill in kbq-form-field is entirely CSS — three rule blocks and four tokens, no TypeScript — and it has regressed five times (DS-2873, DS-4060, DS-4667, DS-4950, DS-4958) with no automated coverage. jsdom implements neither :-webkit-autofill nor CSS animations, so this has to be Playwright. :-webkit-autofill cannot be produced synthetically: choosing a suggestion happens in browser chrome, and the CDP Autofill domain is compiled into Chrome-branded builds only, so it is absent from the Chromium that Playwright bundles. e2eForceAutofill forces the pseudo-class over CDP instead. Chrome applies its own autofill background to a forced element too, so the design system's suppression of that background is genuinely exercised. Only the standard "autofill" spelling is accepted by CSS.forcePseudoState; the legacy name and unknown names alike resolve successfully and force nothing, so the helper verifies through an independent probe stylesheet rather than trusting the call. Forced states are keyed to the node id they were set on and DOM.getDocument re-issues ids, so the session and the root node are cached per page. These tests characterise what main does today. Several of them pin behaviour that is wrong; each is marked DS-4096 and says what the value should become: - the container tint out-ranks the error, disabled, focused, in-overlay and no-borders backgrounds, because the :has() block paints with !important - an autofilled textarea paints Chrome's own opaque background, since .kbq-textarea is in none of the three blocks — glaring in the dark theme and nearly invisible in the light one - no tag input gets the focus geometry: the canonical one carries .kbq-input but tag-list.scss declares min-height: unset !important, and the bare one never had the class - -webkit-text-fill-color is state-blind, so the error and disabled text colours are lost - the inset box-shadow paints nothing, because the token is shadowed to transparent on the control one line above it - the autofill border-color and placeholder tokens are declared, never read The CSSOM group pins the selectors, the !important and the cascade order without needing the pseudo-class to match, so it survives a Chromium that drops the forcing. Screenshots must be taken with animations: 'allow'. The project default finishes every finite animation, and the 5000s background-color transition that hides Chrome's background is finite; fast-forwarding it is irreversible within the page, so the guard runs before the capture rather than after.
…4096)
The obvious objection to the disabled row is that a disabled control cannot
be autofilled, and it cannot: Chrome skips disabled fields when it fills. The
combination is reached from the other side — filled first, disabled after —
which is what conditional forms do ("same as billing address", "use the saved
card"). The pseudo-class is cleared when the value changes, not when the
control is disabled.
Recorded as reasoning rather than measurement, because real autofill cannot
be driven in the Chromium Playwright bundles and forcing the pseudo-class
says nothing about persistence.
Also notes why it is the worst of the four: the tint eats the disabled
background and -webkit-text-fill-color eats the grey disabled text, so an
autofilled disabled field reads as an editable one.
DS-4060 painted the container's `background-color` with `!important` to get the autofill tint to land. An important author declaration out-ranks every normal one, so from then on autofill beat the state the field was in: an invalid field lost its error background and printed error text in the contrast colour, a disabled one lost the grey that says it cannot be edited, and a field in an overlay lost its card background. The rule has carried `// todo quick fix … Technical debt DS-4096` ever since. Autofill says "the browser filled this in" and nothing else, so it now contributes a tint and leaves border, focus ring and text to the state. The tint moves from `background-color` to `background-image`, which is what makes that true rather than merely intended: the state keeps `background-color`, the tint composites over whatever it resolved, and the two can no longer be put in conflict — no `!important`, no priority order to maintain, and `_in-overlay` did not have to learn that autofill exists. Text is repainted per state. `_kbq-form-field-state()` emits its own `:-webkit-autofill` rule, so ordinary specificity picks the right colour and an autofilled invalid field keeps its error text. `.kbq-textarea` joins the two rules it was missing from. It was not merely unstyled: nothing suppressed Chrome's own background, so an autofilled textarea painted the browser's opaque blue — in the dark theme, a near-white block with dark text. In the light theme that colour happens to resemble the intended tint, which is how it survived DS-4958. The focus-geometry block from DS-4950 is deleted. It shrank the autofilled control by twice the outline width because the control's background painted over the 1px inset ring; the control has had no background of its own since that same commit made it transparent, and the tint now lives on the container under a shadow drawn above it. Deleting it also ends the tag-input gap, which `tag-list.scss`'s `min-height: unset !important` would otherwise have kept open. `packages/components/tags` is untouched. The characterization suite from the previous two commits is updated in place, so every intended behaviour change shows up as a test diff: 40 tests and four regenerated baselines. BREAKING CHANGE: three form-field autofill tokens are removed. `--kbq-form-field-states-autofill-border-color` aliased the focus colour and made a merely filled field read as focused; `--kbq-form-field-states-autofill-placeholder` could never be seen, because an autofilled field has a value; `--kbq-form-field-states-autofill-text` forced one colour on every state, which is what lost the error and disabled text colours. Nothing in the repo read the first two, and the third is replaced by the per-state text tokens. `@koobiq/design-tokens` still publishes all four names and deprecates them on its own schedule. `--kbq-form-field-states-autofill-background` is unchanged.
…#DS-4096) The theme is a class, and a class tells the browser nothing. With `color-scheme` left at `normal`, Chrome renders every surface it paints itself from the light palette, however dark the application looks. Autofill is where that surfaces. Refocusing a filled field reopens the autofill popup, and in that state Chrome paints the highlight and the text itself — over the author styles that normally hide it. In a dark form the result is a light block with black text. Nothing in the CSSOM shows it: the suppression is still running, `-webkit-text-fill-color` is unchanged, the container still carries the tint. Measured through a focus/blur/refocus recording, every computed value was byte-identical across the transition; the paint simply never goes through the cascade. The readout that does show it is `color`, which the UA forces on an autofilled control and picks from the used `color-scheme`: black under `normal`, white under `dark`. Both new tests assert it. Declared on the form field rather than the root. `color-scheme` inherits, so this covers every control inside the field and leaves the scrollbars and native controls of the surrounding application alone — whether an application opts in globally is its own decision, not this component's. Verified that the scoped declaration flips the palette exactly as a root-level one does. No baseline changed.
…eating it (#DS-4096) Three corrections after comparing this branch against the earlier attempt on fix/DS-4096, which reached the same fix independently and got these parts right. Text repaint. Emitting the `:-webkit-autofill` rule inside `_kbq-form-field-state()` looked tidy and compiled badly: the mixin runs five times, one of them nested under eight `kbq-form-field-type-*` classes and a `:not(:has(...))`, so two declarations became 84 selectors — 15% of the component's CSS — and gave a text colour a specificity of (0,7,0). A library cannot ship that: overriding autofilled text would have needed the `!important` this change exists to remove. The state now publishes `--kbq-form-field-resolved-text` next to the `color` that reads it, and one (0,3,0) rule reads it back. Same resolved colours, 84 selectors down to 3. Transition duration, 5000s to 600000s. The suppression holds only while the transition runs, and 5000s is 83 minutes — a page left open over lunch reaches the end, lands on the browser's own background, and cannot recover without a reload. Both spellings of the pseudo-class, through `:is(:autofill, :-webkit-autofill)`. The list is forgiving, so each browser keeps the one it knows; a plain comma list would be invalidated whole by the other. The test that pinned legacy-only existed to force this decision and is replaced by one asserting both are present. The three tokens the previous commit removed are restored as declarations that nothing reads, each with a comment saying so, which drops the breaking change entirely. Removing them locally never removed the names anyway — `@koobiq/design-tokens` publishes all four globally and deprecates them on its own schedule — so the only real effect was that overriding them stops working, and that is worth a note rather than a major version. The test that guards this asserts reads rather than declarations and needed no change. No baseline moved.
…(#DS-4096) Corrects ed9c26b, which put `color-scheme` on `.kbq-form-field`. Both rules there landed on the same specificity, so the dark one won on source order under any `.kbq-dark` ancestor — however near a `.kbq-light` was. A light subtree inside a dark application got the dark palette. Nested themes are supported and used: the filter-bar date pipes and the shadow-DOM toast dev app both set a theme class below the root. Declared once per theme class in `kbq-core-theme()` instead. `color-scheme` inherits, so the nearest ancestor decides and the inversion cannot happen. It also belongs there on the merits: the property covers every surface the browser paints for itself — scrollbars, native controls, spellcheck, the autofill highlight — and none of those are a form field's business. The design system already modelled this and never wrote it out: `KbqThemeService` carries `colorScheme` on each theme and exposes a `colorScheme()` computed. No baseline moved, across the whole suite.
…S-4096) `kbqInjectAutofilled()` wraps the CDK's `AutofillMonitor` as a signal, and every control the browser can fill exposes it: `KbqInput`, `KbqInputPassword`, `KbqTextarea`, `KbqTagInput`, and `KbqTagList` forwarding from the input it hosts. `KbqFormField` reflects it as `kbq-form-field_autofilled`. Ported from the earlier attempt on fix/DS-4096, where it was written well. `@angular/cdk` is already a direct dependency and a peer of the package, so this only reaches for an entry point that was not used before. The stylesheet does not key on the class. That is the one deliberate difference from the branch this comes from, which used it as a second arm of the tint selector: the class arrives a frame or two after the paint, and two arms that must agree can drift — a class left on a detached or re-attached control would tint a field the browser no longer considers autofilled. CSS matches `:autofill` directly and remains the only thing that paints; the class is an API for application code and a marker in the DOM. A test asserts no rule references it. Writing the tests corrected an assumption inherited from that branch: forcing the pseudo-class over CDP *does* drive the monitor, because the CDK keys its detection keyframe on `:-webkit-autofill` itself. The e2e suite now asserts both halves are live, and the previous claim that the monitor cannot be reached synthetically is gone. Adds `autofill.spec.ts` — seven unit tests dispatching `animationstart` by hand, the technique the CDK's own tests use, since jsdom has neither the pseudo-class nor CSS animations. That is the practical argument for the signal: this half now costs 8 seconds to verify instead of a Docker run. API guard updated for core, form-field, input, tags and textarea; every entry is additive. No baseline moved.
Autofill was undocumented: nothing told a consumer that an autofilled field is tinted, that the tint is themable, or that there is a signal to read it from. Five tickets have now touched this behaviour without a line of user documentation between them. Written for the layer model rather than adapted verbatim from the earlier attempt on fix/DS-4096, whose section describes autofill as the weakest state and lists tokens this branch no longer reads. Covers what it does, why the tint composites instead of replacing — so an invalid field stays red and tinted and an overlay keeps its card background — that border, focus ring and text belong to the state, the single token with an override example, `color-scheme`, and the `autofilled` signal. It also says plainly that the browser draws its own autofill popup over author styles and that `color-scheme` is the only lever on it, because that is the part users will otherwise file as a bug.
…(#DS-4096) Three descriptions written across this branch say something the code does not do. None of them changes behaviour; all three are the kind of thing the next person debugging this area would trust over the code itself. `form-field.en.md` and its Russian twin credited the CDK's `AutofillMonitor` with adding `kbq-form-field_autofilled` to `<kbq-form-field>`. It does neither: the monitor watches the control's own element from inside `kbqInjectAutofilled()` and yields a boolean signal, and the class exists only because `KbqFormField` reflects that signal through a host binding of its own. A reader who goes looking for the class in CDK code finds nothing. `_form-field-theme.scss` justified the container's single-arm selector by pointing at `:hover` and `:focus` variants "on the control below" — which the commit that wrote the sentence had already collapsed into one `:is(:autofill, :-webkit-autofill)`. It now says what the three arms were and why none of them earned its place, which is the question a reader arrives with. `e2e.ts` described the `kbqFocused` matrix row as covering "the second branch of the same selector" — which reads as dead weight now that the selector it names carries nothing but `z-index`. The row guards against reinstating DS-4950's autofill geometry, deleted in this branch, and now says so.
…m (#DS-4096)
`no autofill rule declares !important` read only `document.styleSheets` and
asserted the `!important` subset was empty, without ever asserting it had found
an autofill rule at all. A rule set that comes back empty — styles moved
somewhere it does not look, which is precisely why the sibling helper in this
file already reads `adoptedStyleSheets` too — reports "no `!important`" exactly
as loudly as a clean stylesheet does, and `!important` is the whole of what
DS-4060 got wrong. It now goes through `readRules`, hoisted to the suite scope
for the purpose, and asserts the set is non-empty before the negative.
`the text repaint is one low-specificity rule` bounded the wrong number.
`classesInSelector` matched `.class` tokens alone, so the `:is()` the rule turns
on contributed nothing, and `split(',')` cut the selector list at the comma
*inside* `:is(:autofill, :-webkit-autofill)` — between them reporting 2 for a
selector the comment beside it calls (0,3,0). A `:hover` appended to that rule
would have climbed real specificity back towards the (0,7,0) this branch
rejected the mixin approach for, while the count stood still.
Arguments of a functional pseudo-class are now dropped before the split — it
takes the specificity of its most specific argument, which is a single class or
pseudo-class in everything this component emits, so the `:is()` counts once and
its arguments do not count again — and pseudo-classes count. The selector
measures 3, the bound is `<= 3`, and tightening it to 2 fails with
`Received: 3`: there is no slack left in it.
All 48 non-screenshot tests in the suite pass. No baseline moved; nothing here
changes what is rendered.
|
Visit the preview URL for this PR (updated for commit 876597e): https://koobiq-next--prs-1908-khranrtk.web.app (expires Thu, 20 Aug 2026 17:54:17 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class autofill support to kbq-form-field by treating browser autofill as a visual tint layer (not a field “state”), while also exposing an autofilled signal hook for application code across relevant controls.
Changes:
- Add
kbqInjectAutofilled()helper +autofilled?: Signal<boolean>support onKbqFormFieldControl, and implement it inKbqInput,KbqInputPassword,KbqTextarea,KbqTagInput(forwarded viaKbqTagList). - Rework form-field autofill styling: suppress UA autofill fill on controls and tint the container via
background-image, keeping existing state colors/borders/rings intact. - Add Playwright + unit coverage for both the CSS behavior and the TypeScript autofill signal, plus e2e utilities to reliably force
:autofillin Chromium.
Reviewed changes
Copilot reviewed 30 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/public_api_guard/components/textarea.api.md | Public API snapshot: exposes KbqTextarea.autofilled signal. |
| tools/public_api_guard/components/tags.api.md | Public API snapshot: exposes KbqTagInput.autofilled and KbqTagList.autofilled. |
| tools/public_api_guard/components/input.api.md | Public API snapshot: exposes KbqInput.autofilled and KbqInputPassword.autofilled. |
| tools/public_api_guard/components/form-field.api.md | Public API snapshot: adds KbqFormField.autofilled and KbqFormFieldControl.autofilled. |
| tools/public_api_guard/components/core.api.md | Public API snapshot: exports kbqInjectAutofilled(). |
| tools/cspell-locales/ru.json | Adds RU spellings related to autofill wording used in docs/comments. |
| tools/cspell-locales/en.json | Adds “autofilled” to spelling dictionary. |
| packages/e2e/utils/index.ts | Re-exports new autofill Playwright helpers. |
| packages/e2e/utils/autofill.ts | Adds CDP-based helpers to force/clear :autofill and probe CSS behavior reliably. |
| packages/e2e/routes.ts | Registers new E2eFormFieldAutofill fixture route. |
| packages/components/textarea/textarea.component.ts | Implements autofilled signal in KbqTextarea via kbqInjectAutofilled(). |
| packages/components/tags/tag-text-control.ts | Extends tag text control contract with optional autofilled signal. |
| packages/components/tags/tag-list.component.ts | Forwards autofill state from the registered tag input via a computed signal. |
| packages/components/tags/tag-input.ts | Implements autofilled signal in KbqTagInput via kbqInjectAutofilled(). |
| packages/components/input/input.ts | Implements autofilled signal in KbqInput via kbqInjectAutofilled(). |
| packages/components/input/input-password.ts | Implements autofilled signal in KbqInputPassword via kbqInjectAutofilled(). |
| packages/components/input/e2e.ts | Removes stale “autofill” state from input e2e matrix (coverage moved to form-field suite). |
| packages/components/form-field/form-field.ts | Adds kbq-form-field_autofilled host class and autofilled getter API. |
| packages/components/form-field/form-field.scss | Removes legacy autofill-specific focus geometry adjustments (no longer needed). |
| packages/components/form-field/form-field.ru.md | Documents autofill tint behavior and the TS autofilled hook (RU). |
| packages/components/form-field/form-field.en.md | Documents autofill tint behavior and the TS autofilled hook (EN). |
| packages/components/form-field/form-field-tokens.scss | Clarifies and deprecates non-background autofill tokens; keeps background tint token. |
| packages/components/form-field/form-field-control.ts | Adds optional autofilled?: Signal<boolean> to the KbqFormFieldControl contract. |
| packages/components/form-field/e2e.ts | Adds E2eFormFieldAutofill fixture rendering state/control matrices for coverage. |
| packages/components/form-field/e2e.playwright-spec.ts | Adds extensive Playwright assertions + screenshots for autofill styling and token/cascade behavior. |
| packages/components/form-field/autofill.spec.ts | Adds unit tests validating the autofilled signal behavior and the form-field host class. |
| packages/components/form-field/_form-field-theme.scss | Reworks autofill CSS: suppress UA fill via transition and tint container via background-image. |
| packages/components/core/styles/theming/_theming.scss | Sets color-scheme for .kbq-light / .kbq-dark so UA autofill surfaces follow the active theme. |
| packages/components/core/common-behaviors/index.ts | Exports new autofill common behavior helper. |
| packages/components/core/common-behaviors/autofill.ts | Implements kbqInjectAutofilled() using CDK AutofillMonitor and returns a readonly signal. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
No description provided.