USWDS - Input: Move input width classes out of usa-form package #6232
Conversation
- remove overriding max-width style on usa-input, usa-input group - minimize impact on range and select
There was a problem hiding this comment.
question: Is this the best location for this file?
There was a problem hiding this comment.
One potential alternative is to move these to the properties file, although most of these values are set using system tokens.
amyleadem
requested review from
CTGM-Bixal,
cathybaptista,
mahoneycm,
mandylloyd and
mejiaj
mahoneycm
left a comment
mahoneycm
left a comment
There was a problem hiding this comment.
This is all looking and working pretty well @amyleadem ! I left a couple thoughts in comments but nothing that is necessarily a blocker.
There was a problem hiding this comment.
One potential alternative is to move these to the properties file, although most of these values are set using system tokens.
packages/usa-input-prefix-suffix/src/styles/_usa-input-prefix-suffix.scss
Show resolved
Hide resolved
- Prevent overrides on usa-input--[width] classes
mahoneycm
left a comment
mahoneycm
left a comment
There was a problem hiding this comment.
LGTM! No perceived regressions from develop
Testing checklist
- Confirm the
.usa-input--[width]and.usa-input-group--[width]classes match the CSS (class names and values) in develop. - Confirm
usa-input--[width]andusa-input-group--[width]classes work as expected both with AND without a.usa-formparent on the following elements:- [ ] Confirm no regressions as a result of removingmax-width: nonefrom.usa-form . usa-inputand.usa-form . usa-input - Confirm that creating
$system-input-widthsinuswds-coremakes sense
There was a problem hiding this comment.
Nice work, the new features are a nice enhancement that avoid breaking changes. Also, shoutout to @mahoneycm for the thorough review.
I tested:
- Updating
$theme-input-max-widthand ensure there aren't conflicts. - Different sizes for input group for input prefix/suffix.
- Different sizes for input widths
- Compared against
develop
packages/usa-input-prefix-suffix/src/styles/_usa-input-prefix-suffix.scss
Show resolved
Hide resolved
mandylloyd
left a comment
mandylloyd
left a comment
There was a problem hiding this comment.
Confirmed all the testing and review steps and confirmed previous requested changes look good. Approving!
|
@mlloydbixal @mahoneycm (cc: @aduth) I've updated the Sass to generate explicit |
This reverts commit d6d2eb4.
mahoneycm
left a comment
mahoneycm
left a comment
There was a problem hiding this comment.
This is working great! Great job covering all test cases and maintaining the current behaviors.
✅ ✅ ✅
Testing checklist
- Confirm the
.usa-input--[width]and.usa-input-group--[width]classes match the CSS (class names and values) indevelop - Confirm
usa-input--[width]andusa-input-group--[width]classes work as expected both with AND without a.usa-formparent on the following elements:-
usa-input -
usa-textarea -
usa-input-group - Character count input (currently accepts the
usa-input--[width]classes indevelop) - Input mask input (currently accepts the
usa-input--[width]classes indevelop)
-
- Confirm no regressions as a result of removing
max-width: nonefrom.usa-form . usa-inputand.usa-form . usa-input- Looks good. The max width from
usa-formmanages the max width without the need to set it individually on nested elements. - Should we remove range and select?
- Looks good. The max width from
- Confirm that creating
$system-input-widthsinuswds-coremakes sense
amyleadem
requested review from
heymatthenry and
thisisdano
and removed request for
aduth
thisisdano
left a comment
thisisdano
left a comment
There was a problem hiding this comment.
Nice work, this one got a bit complicated!
Summary
Moved
.usa-input--[width]and.usa-input-group--[width]classes out of theusa-formpackage.These classes are now generated in the
usa-inputandusa-input-prefix-suffixpackages and can be used without the.usa-formparent element.Note
This is an alternative solution to PR #5935 (Thanks @aduth!)
Breaking change
This is not a breaking change.
Related issue
Closes #5312
Related pull requests
Changelog PR
Preview link
Problem statement
As a developer, I expect that if I include the package listed on a component's documentation (e.g. Text Input), all of the usage options documented for that component are available, so that I'm not confused by styles not existing despite being documented, or be forced to include more packages than I'd otherwise need for my optimized installation.
Solution
.usa-input--[width]and.usa-input-group--[width]class defintions into theusa-inputandusa-input-prefix-suffixpackage, respectively.$system-input-widthsmap in theuswds-corepackage so that the values can be defined in a single location$system-input-widthsmap to generateusa-input--[width]andusa-input-group--[width]classesmax-width: nonedeclaration from.usa-form .usa-inputand.usa-form .usa-textareato prevent specificity overridesTesting and review
.usa-input--[width]and.usa-input-group--[width]classes match the CSS (class names and values) indevelopusa-input--[width]andusa-input-group--[width]classes work as expected both with AND without a.usa-formparent on the following elements:usa-inputusa-textareausa-input-groupusa-input--[width]classes indevelop)usa-input--[width]classes indevelop)max-width: noneon.usa-formchildren:$theme-input-max-widthto a value less than mobile (like "card" or 15)$system-input-widthsinuswds-coremakes sense and lives in the appropriate location