feat(text-field): accept exceptionallySetClassName on the input element#1047
feat(text-field): accept exceptionallySetClassName on the input element#1047lmjabreu wants to merge 1 commit into
Conversation
Lets callers attach a className to the underlying <input> as an escape hatch when the design system can't yet express a needed styling hook. Mirrors the convention already used by Box, Tooltip, Loading, etc. Why this is needed: the deprecated <Input> component accepted a className on the underlying input element. Two real callsites depend on that affordance with no clean rewrite (an Adaptive Cards host that targets the input by class, and a borderless popover input). See ADR for full reasoning and the Twist discussion that established the convention. Decision: https://github.com/Doist/component-libraries/blob/main/decisions/2026-05-20-exceptionally-set-classname.md Twist thread: https://twist.com/a/1585/ch/81455/t/7796256/ Scope notes: - SelectField is intentionally excluded for now per the ADR caveat (the FormSelect/Adaptive Cards callsite is moving with a separate cleanup, not migrating). - BaseField needs no change; it does not directly render the inner field element. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
left a comment
There was a problem hiding this comment.
This PR thoughtfully introduces the exceptionallySetClassName prop to TextField, providing a deliberate escape hatch for styling the underlying input element and unblocking ongoing migration efforts. While the implementation aligns well with the design system's styling conventions, a couple of adjustments are needed to ensure the TypeScript documentation accurately reflects that the class targets the nested input rather than the container. Additionally, the testing approach should be refined to query the input by role rather than relying on a test ID to guarantee the assertion targets the exact element.
| BaseFieldVariantProps, | ||
| Pick<BaseFieldProps, 'characterCountPosition'> { | ||
| Pick<BaseFieldProps, 'characterCountPosition'>, | ||
| ObfuscatedClassName { |
There was a problem hiding this comment.
[P2] Extending ObfuscatedClassName here makes the public prop docs say this class is applied to the component's main container, but this component now forwards it to the nested input instead. That leaves the TypeScript/IDE contract wrong for both TextField and PasswordField. Define exceptionallySetClassName locally on TextFieldProps with JSDoc that says it targets the underlying input, rather than inheriting the shared root-element type.
| exceptionallySetClassName="custom-input-class" | ||
| />, | ||
| ) | ||
| expect(screen.getByTestId('text-field')).toHaveClass('custom-input-class') |
There was a problem hiding this comment.
[P2] Using data-testid may target the field's wrapper instead of the underlying <input> element, which undermines the test's intent to verify exactly where the class lands. To guarantee you are asserting on the input element itself (and to follow the project's testing conventions), query by role instead.
expect(screen.getByRole('textbox', { name: 'Whatʼs your name?' })).toHaveClass('custom-input-class')You can then safely remove the data-testid prop from the TextField on line 245.
Closes: no issue (decided via the exceptionallySetClassName ADR, motivated by the Reactist colour audit).
Short description
TextFieldnow acceptsexceptionallySetClassNameand forwards it asclassNameon the underlying<input>element. This mirrors the existing Reactist convention used byBox,Tooltip,Loading,Hidden,Heading,Inline,Stack, etc.The need was surfaced by the DS-P005 migration: two callsites of the deprecated
<Input>rely on a className landing on the input element (an Adaptive Cards host config that targets the input by class, and a borderless popover input that needs the styling hook). Without this affordance the migrations either need per-callsite CSS rewrites or ad-hoc workarounds.The escape-hatch framing in
ObfuscatedClassName's JSDoc still applies — adding the prop is not a green light for callers to style the design system freely; it is a deliberate, named exit ramp for cases the component cannot yet meet on its own.Scope notes
SelectFieldintentionally excluded. Per the ADR caveat from @pawelgrimm, the one currentSelectFieldcallsite that would need this (FormSelect→ Adaptive Cards in comms-web) is being handled by a separate cleanup that may drop the integration entirely. No need to pre-emptively expand the API.BaseFieldneeds no change. It is an abstraction over the children render-prop pattern and does not directly render the inner field element. Forwarding happens in the concrete field component (TextField).PR Checklist
Related work