(inputs): make suffixes and affixes in consistency#4525
(inputs): make suffixes and affixes in consistency#4525ArtemKhvorostianyi wants to merge 20 commits into
Conversation
|
@ArtemKhvorostianyi yes remove bg by default in all inputs for affixes |
|
these 3 places look weird - 2 icons on left, and too much spaces on the right - check this is consitent with other inputs |
rorychatt
left a comment
There was a problem hiding this comment.
A lot more work here is needed.
Also make sure styles are consistent across different sizes.
Staging preview📄 Docs: https://ivy-framework-staging-docs-4525.sliplane.app |
|
@claude review this |
|
Claude finished @rorychatt's task in 2m 30s —— View job Code Review CompleteI've reviewed the PR changes across 37 files. This is a significant refactoring effort to standardize suffix/affix handling across all input components. Overall the architecture is well-designed with shared styling utilities in Issues Found1. Duplicate Icons in DateTime/DateRange InputsFiles: The screenshots show duplicate calendar icons appearing. This happens because:
Suggested fix: When 2. Inconsistent Border Rounding in Affix ShellsFiles: Multiple input widgets Some affix cells have curved edges while others don't. The Looking at the code in hasPrefix && "rounded-l-none",
hasSuffix && "rounded-r-none",
!hasAffixes && "rounded-field",This logic is correct, but ensure all input variants apply it consistently. 3. Text Starting Position Too Far Right (NumberInput)File: The prefix cell applies <div
className={cn(
textInputAffixCellClasses("prefix", density),
textInputAffixIconOnlyPaddingVariant({ density }), // This adds !px-2 on Medium
)}
>Suggested fix: Review whether 4. Select Inputs Have Different Background FillFile: The standalone select trigger (when no affixes) creates its own border container with Looking at line 224-228: !hasAffixes &&
"border-0 shadow-none rounded-field focus:ring-0 focus-visible:ring-0 focus:ring-offset-0 dark:bg-transparent",The 5. Missing Text Overflow EllipsisFiles: From the screenshots, several inputs don't show ellipsis (...) when text overflows. The text input value should have In Code Quality ObservationsPositive
Suggestions
SummaryThe refactoring is architecturally sound but needs refinement to address the visual inconsistencies shown in the reviewer's screenshots. The main issues are:
Once these visual issues are resolved, the PR will provide a much more consistent input component library. | |














Goal:
Demo
Text inputs:

Text inputs with KBD usage:

For text input also:
Select inputs:

Number inputs:

Bool input:

Color input:

Feedback input:

Icon input:

Code input:

DateTime input:

DateRange input:

General comparison of inputs: