fix(input): make leading and trailing slots usable - #230
Merged
Merged
Conversation
The two slots were part of the public API but had no test coverage, and three separate things were wrong with them. isLeading and isTrailing ignored the slots, so the padding compound variants never applied. The same icon measured 36px of padding through the leadingIcon prop and 12px through the slot, leaving the content overlapping the text by 22px. Both flags now account for the slots. The slot wrapper carries pointer-events-none so that a decorative icon stays click through and clicking it focuses the field. That also made interactive slot content impossible to click: a button in a slot never received a single click, while the cursor still changed on hover. Pointer events are now re-enabled only on a wrapper that actually holds a slot, and only while the field is neither disabled nor loading, so a control inside a disabled field stays inert. A slot also took precedence over the loading branch, so a field with a slot went disabled during loading with no spinner and no other indicator. Loading now wins, matching how Button swaps its leading icon for the spinner. Also corrected the loading prop documentation, which claimed it optionally disables interaction while the input is always disabled while loading. That text ships in the published type declarations. Closes #229
8 of 12 tasks
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.
Summary
leadingSlotandtrailingSlotare part of the public API ofInputand had no test coverage at all, which is how three separate defects survived in them. This came out of #228, where the reporter suggested composingInputwith a button as the alternative to a dedicated component. I tried exactly that and it did not work.Measured in a real browser before changing anything, with the supported
leadingIconprop as the control case.The slots reserved no room in the field:
Interactive content in a slot could not be clicked:
The cursor still changed to a pointer on hover, so it looked clickable. The workaround was
ui={{ trailing: 'pointer-events-auto pe-1', base: 'pe-10' }}, which had to be reverse engineered and whose padding value had to be guessed per size.A slot also swallowed the loading spinner. Three inputs in a loading state on one page produced one spinner and three disabled fields.
Closes #229
Type of change
Changes
isLeadingandisTrailingaccount for the slots, so the padding compound variants that already existed now apply. No new classes were added for this.Buttonswaps its leading icon for the spinner.loadingprop documentation, which claimed it optionally disables interaction while the input is always disabled while loading. That text ships in the published type declarations and shows up in editor tooltips.Checklist
Closes #229)pnpm checkpasses (0 errors, 0 warnings)pnpm lintpassespnpm testpassesCHANGELOG.mdunder[Unreleased]*.types.ts, Material 3 design tokens)Notes
input.variants.tsis untouched. An earlier attempt added two boolean variants for the pointer events, which turned out to be unnecessary: the slot function already merges itsclassargument, so a conditional entry in the array that was already there resolves correctly and keepsuioverrides working. Verified that a caller passingui={{ trailing: 'pointer-events-none' }}still wins.Verified on a throwaway page in a real browser. Every wrapper ends up in the right state:
Eight regression tests cover padding on both sides, pointer events on and off, the decorative case, the disabled and loading cases, spinner precedence, and slot rendering when idle. Tests: 3868 passing across 107 files.