-
Notifications
You must be signed in to change notification settings - Fork 6
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
DSD-1623: label spacing for DatePicker, Select, Slider, and TextInput #1562
DSD-1623: label spacing for DatePicker, Select, Slider, and TextInput #1562
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -125,6 +125,10 @@ const labelLegendText = { | |||
color: "dark.ui.typography.heading", | |||
}, | |||
}; | |||
// Used in form inputs that require nuanced spacing. | |||
const labelLegendTextSpecialSpacing = { | |||
marginBottom: "xxxs", |
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.
Why not add this rule to fieldset.ts
and label.ts
? This seems like it'll be easier to miss if the update is in the component file than the style file.
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.
I was following the pattern of labelLegendText
because I wanted a reusable object.
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.
Yes, but labelLegendText
is used in fieldset.ts
and label.ts
and not in the __css
prop for the components. So what's here is fine but if we can keep the update in the theme files, that'd be ideal.
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.
Approving but would like to see this in the theme file at some point if possible.
src/theme/components/datePicker.ts
Outdated
|
||
const DatePicker = defineMultiStyleConfig({ | ||
baseStyle: definePartsStyle({ | ||
fieldset: { | ||
legend: { ...labelLegendTextSpecialSpacing }, |
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 can be legend: labelLegendTextSpecialSpacing,
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.
Update looks good but just needs one minor update.
Fixes JIRA ticket DSD-1623
This PR does the following:
DatePicker
,Select
,Slider
, andTextInput
components to reduce the spacing between the field label and the field itself from"8px"
to"2px"
.FilterBar
andMultiSelectGroup
components were not addressed because they have not yet been reintroduced back into the DS.How has this been tested?
Accessibility concerns or updates
Checklist:
Front End Review: