-
Notifications
You must be signed in to change notification settings - Fork 201
chore(combobox, datepicker,et all): add displayLabel arg #3660
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: spectrum-two
Are you sure you want to change the base?
chore(combobox, datepicker,et all): add displayLabel arg #3660
Conversation
|
🚀 Deployed on https://pr-3660--spectrum-css.netlify.app |
File metricsSummaryTotal size: 1.38 MB* 🎉 No changes detected in any packages * Size is the sum of all main files for packages in the library.* An ASCII character in UTF-8 is 8 bits or 1 byte. |
ca1fb6f
to
2659e83
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definitely feels like the simpler solution and works nicely.
It is interesting to note though, looking through the guidelines they say "text fields should always have a label" and then in something like combobox, the field label can exist but the markup is outside .spectrum-Textfield
(and .spectrum-Combobox
for that matter).
- without the displayLabel arg set to false in the following components, the textfield was still rendering a label, and offsetting a bunch of the icons & buttons within these components combobox, datepicker, form, pagination, search, stepper
2659e83
to
9709150
Compare
Description
After rebasing the search field migration branch, I noticed that the icons were placed awkwardly. I realized that the nested textfield in my search component was just missing a
displayLabel: false
argument. That arg was updated in the textfield S2 migration, to be set totrue
on default (based on guidance that "a textfield should always have a label"), but because search isn't utilizing the field label that ships with textfield, that arg should be set tofalse
. Without the displayLabel arg set to false, the textfield was still rendering an additional, empty field label component, and offsetting the icon & button within search.After scanning the repo, the following components needed similar
displayValue: false
args:combobox, datepicker, form, pagination, stepper
Jira/Specs
CSS-1174
How and where has this been tested?
Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.
Validation steps
spectrum-two
(these components do look broken):Regression testing
Validate:
Screenshots
Before 🚫

combobox
datepicker

search

pagination

form

stepper

After ✅

combobox
datepicker

search

pagination

form

stepper

To-do list