-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[FormControl] Add fullWidth
prop to FormControl
context
#19369
Conversation
Details of bundle changes.Comparing: 1092c22...00753db
|
Looking into the argos failure... I'm not sure why MobileStepper might have a visual difference from these changes. |
@eps1lon @oliviertassinari From what I can tell, the argos failure is unrelated to my changes. I can't see any reason for this failure in the source for MobileStepper (the failing test was for TextMobileStepper) or any of its subcomponents. Seems like maybe the image that's being loaded changed? Really unsure. Can someone help me understand what's going on here? Cheers! |
@@ -224,6 +224,16 @@ describe('<FormControl />', () => { | |||
setProps({ margin: 'dense' }); | |||
expect(formControlRef.current).to.have.property('margin', 'dense'); | |||
}); | |||
|
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 seems like it might be a good place for this additional test, but if there is a better way/location, let me know.
type ContextFromPropsKey = | ||
| 'disabled' | ||
| 'error' | ||
| 'fullWidth' |
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'm not sure this is correct or even necessary, but it seems like a type update would be good to do for anything that returns the updated context object.
@EsoterikStare you're correct – it's an aberration. I've cleared the error. |
@EsoterikStare Great work on the PR. Reasonable feature, added tests, modified types. Nothing more to wish for 👍 |
Per the discussion in #19336, I've added the
FormControl
propfullWidth
to theFormControl
context. I've also added a test to ensure this prop is present in the context and updated the type ofuseFormControl
.I'm also not sure if there is any documentation to update. I don't see anything, but I may have missed something.