-
Notifications
You must be signed in to change notification settings - Fork 599
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
feat: Support className
prop in FormControl.Caption
component
#5726
Conversation
🦋 Changeset detectedLatest commit: 0c0fbcd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
size-limit report 📦
|
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.
Looks great! Just left a comment for the changeset, I think this would fall under minor
for us since this is adding functionality.
Co-authored-by: Josh Black <joshblack@github.com>
What does this PR add?
This PR adds a
className
prop toFormControl.Caption
. Related components—e.g.FormControl
andFormControl.Label
—already have aclassName
prop.Does
FormControl.Caption
need aclassName
prop?Yes! There are basically 3 options for styling
FormControl.Caption
:Option 1: Wrap
FormControl.Caption
in a<div>
, and style the<div>
. This option is a non-starter, because GitHub useseslint(primer-react/direct-slot-children)
to enforce thatFormControl.Caption
is a direct child ofFormControl
.Option 2: Style
FormControl.Caption
usingsx
. This option is a non-starter, becausesx
is deprecated.Option 3: Style
FormControl.Caption
usingclassName
. This option is not possible today, becauseFormControl.Caption
is missing aclassName
prop, but this is otherwise the best way to styleFormControl.Caption
, ergo this PR.Why do you want to style
FormControl.Caption
anyways?The image below gives an example of the kinds of styles that might be applied—in it, a
FormControl.Label
is positioned atopFormControl.Caption
, and these (together) are displayed beside a control:With
sx
, that layout can be accomplished like this:I’d like to accomplish the same layout, using
className
in place ofsx
. This PR makes that possible.Changelog
New
className
prop toFormControl.Caption
componentRollout strategy
Testing & Reviewing
Do other components have
className
-related tests or previews?Merge checklist