Skip to content

feat(text-field): accept exceptionallySetClassName on the input element#1047

Open
lmjabreu wants to merge 1 commit into
mainfrom
feat/textfield-exceptionally-set-classname
Open

feat(text-field): accept exceptionallySetClassName on the input element#1047
lmjabreu wants to merge 1 commit into
mainfrom
feat/textfield-exceptionally-set-classname

Conversation

@lmjabreu
Copy link
Copy Markdown

Closes: no issue (decided via the exceptionallySetClassName ADR, motivated by the Reactist colour audit).

Short description

TextField now accepts exceptionallySetClassName and forwards it as className on the underlying <input> element. This mirrors the existing Reactist convention used by Box, 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

  • SelectField intentionally excluded. Per the ADR caveat from @pawelgrimm, the one current SelectField callsite 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.
  • BaseField needs 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

  • Added tests for bugs / new features (one new test asserting the className lands on the input element)
  • Updated docs (storybooks, readme) — happy to add a story showing the escape hatch if reviewers want; left out for now since the prop is intentionally discouraged.
  • Reviewed and approved Chromatic visual regression tests in CI — pending CI run; no visual diffs expected (the prop is opt-in and unused in stories).

Related work

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>
Copy link
Copy Markdown
Member

@doistbot doistbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Share FeedbackReview Logs

BaseFieldVariantProps,
Pick<BaseFieldProps, 'characterCountPosition'> {
Pick<BaseFieldProps, 'characterCountPosition'>,
ObfuscatedClassName {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants