feat(forms): add isUnified InputGroup variant and ClearableInput component - #2148
Open
jgorfine-zendesk wants to merge 67 commits into
Open
feat(forms): add isUnified InputGroup variant and ClearableInput component#2148jgorfine-zendesk wants to merge 67 commits into
jgorfine-zendesk wants to merge 67 commits into
Conversation
Lets InputGroup be styled and focused like a single bordered field (matching MediaInput) while remaining a group of independent controls, for composite date-field-style layouts in the datepicker bug bash. - InputGroup always renders role="group" and auto-labels itself from the enclosing Field's rendered label (new hasLabel context, ported from the sibling input-variants branch's identical MediaInput fix, so aria-labelledby never dangles when a Label isn't actually mounted) - Input auto-goes $isBare inside a seamless group - StyledInputGroup gains the container border/focus styling, icon-button sizing with focus-ring clearance, and a text-button padding override so spacing stays visually even next to a zero-padding IconButton - exports StyledButton/StyledIconButton from @zendeskgarden/react-buttons so StyledInputGroup can target each by component, not by role/tag guessing - adds an isSeamless story control and a "Clear value" pattern demo Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The z-index stacking rules in itemStyles exist only to manage the non-seamless variant's overlapping child borders. Gate them behind !isSeamless, alongside the existing margin/radius overlap block, so seamless children fall back to normal stacking instead of carrying z-index values with nothing left for them to manage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…font-size padding-inline was computed in em units to match StyledTextInput's own non-bare horizontal padding, but the container never declared a font-size of its own, so the em value resolved against whatever ambient font-size it inherited instead. Add a sizeStyles constant (mirroring StyledTextInput's) that sets font-size unconditionally and owns the seamless variant's padding/gap, so the padding renders at the intended size regardless of isCompact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…iner padding An edge IconButton's own icon sits inset within the button by (button size - icon size) / 2, so combining the container's declared edge padding with the button's own inset pushed the icon in twice as far as the Figma spec calls for. Detect a leading/trailing IconButton with :has() and reduce that side's padding-inline by the button's inset, so the icon itself lands at the container's declared padding regardless of isCompact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…iner padding A leading/trailing text Button already has its own padding overridden elsewhere to match the container's inter-item gap, so combining that with the container's edge padding pushed the button's text in further than intended. Extend the :has() edge compensation added for IconButton to plain text Buttons too, scoped with :not(StyledIconButton) since IconButton extends StyledButton and would otherwise match both rules, matching the "A / B / Content / Copy" InputGroup story. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… height A default-sized text Button is 40px tall, taller than the seamless container's own 40px/32px min-height allows for once its border is accounted for, so an InputGroup with text buttons (the "A / B / Content / Copy" story) rendered 50px instead of 40px. Override height/line-height on the text Button to match the IconButton's own size (28px regular, 24px compact), so it no longer grows the container past its declared min-height. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n-seamless variant focusInset was hardcoded on the story's Button/ToggleButton regardless of isSeamless. The non-seamless variant still needs it to avoid the ring clipping against overlapping borders, but the seamless variant gives each button its own space, so it should show the default outward ring instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Previews the layout planned for the new DatePicker: a seamless InputGroup with two trailing IconButtons (clear, then a calendar trigger), to validate that two icon buttons sitting next to each other at the end of the group keep even spacing and focus rings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The seamless container previously shrunk IconButton to a custom 28px (24px compact) via CSS overrides to make it fit, rather than letting it render at its own regular size as the design intends. IconButton now stays at its own "small" size, with the container's own vertical padding and edge-padding compensation recomputed to fit that. A regular "small" IconButton (32px) fits a 40px container with room to spare, but not a 32px compact container plus its own border - that case still needs a small CSS-forced shrink (32px -> 30px) to fit, and an inset focus ring since there's no padding-block left for an outward one to clear. Since a seamless container's math depends on IconButton actually being "small" with an inset ring, InputGroup now forces both props on any IconButton child whenever isSeamless, rejecting - not just filling in - any explicit size/focusInset the consumer passes, since any other value would break the layout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DatePicker needs a different InputGroup approach, so drop this story ahead of that work. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…story InputGroup's isSeamless container ring now supports an explicit focusInset, and gains a hover/transition border-color to match Input. Also fixes the seamless container's align-items to stretch instead of center. Adds a "Date field" pattern story demonstrating a nested InputGroup layout ahead of the DatePicker work. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
An explicit aria-label should take precedence over the label auto-derived from Field context, rather than losing to it per ARIA's aria-labelledby precedence rules. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lidation StyledTextInput now reflects its $validation as a data-validation attribute (alongside the existing boolean aria-invalid), so a sibling isSeamless InputGroup can select on it via :has() and color its own border/focus ring to match success/warning/error - without needing a validation prop on InputGroup itself. Also updates the "Date field" pattern story to demonstrate this: it's now a required field that shows an inline error and colors the seamless segment red when blurred empty, and drops the label/disabled/hasHint/isDragging controls that don't apply to this pattern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d their glyph to 12px A "small" IconButton (32px) barely needed to shrink at all to fit a compact seamless container, and its icon glyph never shrank alongside it - both made isCompact read as basically a no-op. Icon buttons now shrink to 28px and their glyph to iconSizes.sm (12px), with the edge-padding inset math updated to match. Also threads isCompact through the Date field pattern story's nested InputGroup, Input, and calendar IconButton, none of which auto-resize from the outer group's isCompact alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Buttons/icon buttons sitting off-center within the container's padding looked wrong once the container centered its children by default; instead only the text input stretches to the container's full height, everything else stays centered. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extracts the clear-button InputGroup pattern from the Date field demo story into a reusable, accessible ClearableInput, and switches that story to use it.
Keeps the default short and generic since it's fully overrideable via clearButtonLabel/useText; updates the Date field story's override to match.
Tracks whether the clear button should render via local state synced from real input change events, rather than the value prop alone, so ClearableInput works whether the consumer supplies value/onChange or just defaultValue. Also adds a matching Uncontrolled story alongside Controlled.
Composes onChange/onClick handlers inline in JSX (matching the mergeRefs convention used elsewhere), and folds onClear into the same onClick composition instead of calling it manually. Also narrows value/defaultValue to string, since a native input's value is always a string at runtime, letting the has-a-value checks drop their unneeded String() coercion.
jgorfine-zendesk
force-pushed
the
jgorfine/datepicker-bug-bash/1.5/input-group-variant
branch
from
August 28, 2026 03:01
8f6f52e to
90e6dbd
Compare
Comments added while test-driving the InputGroup/ClearableInput work were longer and more frequent than this codebase's usual terse style. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The new ClearableInput component now covers this pattern directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The seamless InputGroup only transitioned border-color, so its focus box-shadow snapped in/out instead of easing like MediaInput's does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…aves the whole group Activating the clear button blurred the inner Input directly, which flagged the field as touched even though focus was just moving to a sibling button in the same InputGroup. Track blur on the outer group instead, only treating it as "touched" once focus actually leaves it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The clear button's aria-label already announces its purpose, so the decorative icon inside it shouldn't also be exposed to screen readers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ze-flo
reviewed
Sep 2, 2026
Move role="group" before the props spread so consumers can override it, defaulting to "group" only when unspecified.
…Group variant Gate the disabled-button border-top/bottom-width reset behind the non-unified case, since unified children no longer overlap and the unconditional rule left floating vertical border lines. While here, consolidate the segmented/unified conditionals into dedicated segmentedItemStyles/unifiedItemStyles mixins selected at the call site, mirroring each other.
…ifiedItemStyles sizeStyles' font-size declaration only grounds the unified variant's em-based padding math; the segmented variant never used it, since every descendant (StyledTextInput, Button, IconButton) sets its own explicit font-size. Drop the vestigial declaration from the segmented path and fold sizeStyles entirely into unifiedItemStyles, selected by the same isUnified condition as the other item mixin, leaving segmentedItemStyles untouched.
[InputGroup](#inputgroup) and [FauxInput](#fauxinput) link to anchors that don't exist on the Garden docs page; drop the link syntax and keep the plain component name.
Mirrors Input's shared propTypes (isCompact, isBare, focusInset, validation) plus ClearableInput's own props, matching the runtime prop validation every other exported component declares.
…per clicks to the input isUnified and isCompact now have a single source of truth (ClearableInput's own props), matching the mental model that isCompact/isUnified are whole-component concerns, not wrapper-only escape hatches via wrapperProps. focusInset instead follows MediaInput's convention: settable at the top level, still overridable via wrapperProps.focusInset. Clicking anywhere in the wrapper now focuses the input, composing with any wrapperProps.onClick. Also exposes a focusInset control on the ClearableInput story.
…rProps type
Neither is respected via wrapperProps at runtime: isUnified is always true, and isCompact's single source of truth is ClearableInput's own top-level prop. The type shouldn't advertise either as settable through wrapperProps. Keeps the Omit/blocklist shape (rather than Pick-ing an allowlist like IFauxInputProps does) since wrapperProps is intentionally an arbitrary HTMLAttributes passthrough, tested via wrapperProps={{ className: ... }}. Marks the two affected runtime-lock-down tests with @ts-expect-error, since they now intentionally pass values outside the narrowed type to verify the lock-down survives a consumer bypassing the type checker.
isBare previously flowed only to the inner Input, which is already forced bare by unified-context defaults, making it a no-op. Add a StyledClearableInput that wraps InputGroup and, when $isBare, strips the outer border/radius/background and suppresses the focus-within ring on the underlying StyledInputGroup via a same-element compound selector (&&${StyledInputGroup}), scoped to ClearableInput only rather than growing InputGroup's own public isBare surface.
isBare is a single source of truth (like isCompact/isUnified), not dual-path like focusInset, since there's no independent "wrapper-only bare" concept the way focusInset has two genuinely different ring targets.
InputGroup.tsx now imports StyledInputGroup directly rather than through the styled barrel, to avoid a circular import once the barrel also exports StyledClearableInput (which needs InputGroup).
…s.onClick onClear duplicated buttonProps.onClick, since ClearableInput already composes buttonProps.onClick with its internal clear handler. Consumers who want to observe a clear should use buttonProps.onClick instead.
…not data-validation attr Selecting on a descendant's data-validation attribute meant StyledInputGroup had to know about StyledTextInput's DOM shape, and only worked for direct text-input descendants (e.g. not Textarea). Instead, Input now publishes its validation to the nearest unified InputGroup via context, and InputGroup derives its own $validation prop from that, mirroring StyledTextInput's own border color model. A unified group supports a single validation-bearing Input; a second one warns in development and the map-based registry falls back to last-registered on unmount. Co-authored-by: Florent Mathieu <florent.mathieu@zendesk.com>
…-down test Unlike isCompact/isUnified, isBare was never part of InputGroup's public props, so there's no real override path for wrapperProps to guard against. The test could never fail; its only observable effect was a harmless React DOM warning from the @ts-expect-error prop injection reaching the DOM.
… on mousedown A mouse click on the clear button shifted focus to the button before the click cleared the value, blurring the input with its stale (pre-clear) value in between. Consumer onBlur handlers (e.g. touched-state tracking) would see the old value, then a separate onChange with the cleared one. Compose buttonProps.onMouseDown with an internal handler that calls preventDefault, consumer-first, so the button never takes focus on mouse interaction and the input keeps focus through the whole mousedown/click sequence. Enter/Space activation is unaffected, since keyboard activation never dispatches a mousedown. Co-authored-by: Florent Mathieu <florent.mathieu@zendesk.com>
…t in unified InputGroup An IconButton (e.g. ClearableInput's clear button) sat flush against a preceding Input, since neither the Input nor the container's own edge padding reserved a gap for it there — unlike a plain Button, which already gets this via the generic `* + Button` sibling margin. Most noticeable with autofill highlighting, or once the button shows a focus ring, since the button's box (not just its icon) had no breathing room from the input.
ze-flo
approved these changes
Sep 3, 2026
…ledMessage A consumer (e.g. DatePicker's calendar popover wrapper) may render its own hidden element directly after a unified InputGroup, which defeats the adjacent-sibling (+) match used to space the group from a trailing StyledMessage. Use the general-sibling (~) combinator there instead.
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.
Description
Adds an
isUnifiedvariant toInputGroupthat renders as a single bordered/focused field (like the GardenInput) instead of the classic overlapping-border segmented look, plus afocusInsetprop for insetting that container's own focus ring (e.g. when nested inside another group). The unified container now also:data-validationattribute onStyledTextInputselected with:has()- novalidationprop needed onInputGroupitselfInput, and transitions its focus box-shadow at the same timing asMediaInputIconButton,ToggleIconButton, …) to fit the container by targeting react-buttons' publicdata-garden-idhooks via:has()/descendant selectors — no newsizevariant is added toIconButton; in the compact case the button is shrunk further, while its icon glyph stays aticonSizes.mdAlso fixes
InputGroupto omit its auto-derivedaria-labelledbywhen an explicitaria-labelis provided.Adds a new
ClearableInputcomponent built on the unifiedInputGroup: anInputpaired with a clear button that appears once there's a value to clear. Supports both controlled and uncontrolled usage, returns focus to the input when the clear button is activated (even though the button unmounts as a result), and defaults the clear button'saria-labelto "Clear" (the icon itself is hidden from assistive technology).Adds a "Date field" pattern story demonstrating a nested
InputGrouplayout (an inner unifiedClearableInputalongside a separate bordered calendarIconButton) as a step toward the upcomingDatePickercomponent, including a required-field validation example that only flags the field once focus leaves the whole group, not just the inner input.Checklist
npm start)?bedrock)