refactor: [M3-7562] - Remove Radio.mdx and Textfield.mdx files #9994
Conversation
| - Avoid breaking a single form into multiple “papers” unless those sections are truly independent of each other. | ||
| - Consider sizing the input field to the data being entered (ex. the field for a street address should be wider than the field for a zip code). Balance this goal with the visual benefits of fields of the same length. A somewhat outsized input that aligns with the fields above and below it might be the best choice. | ||
|
|
||
| ## Textfield errors |
There was a problem hiding this comment.
This documentation section corresponds to the error textfield story - had to add it here instead of above the error textfield story to get it to show up in the storybook documentation section. We removed showing all stories in the documentation (see here).
| Before | Current (see comment above for changes) |
|---|---|
 |
 |
| @@ -73,6 +73,10 @@ interface BaseProps { | |||
| * className to apply to the underlying TextField component | |||
| */ | |||
| className?: string; | |||
| /** | |||
There was a problem hiding this comment.
The textfield component is currently in the root of the components package - thoughts on moving it into a separate TextField folder within components? (this will lead to a way bigger file diff - like +80 files due to all the import changes though 😅 )
There was a problem hiding this comment.
Thanks for considering this. In the past we had components/core/ and we migrated away from that to just having components/. I don't seem to recall if we have guidelines to determine how things should be organized. At the moment it appears to be a mixture of individual folders and files. I would prefer that each component is contained within a designated folder containing tests and storybook files.
This might be good to discuss during the Cafe.
There was a problem hiding this comment.
I think this could be done separately if we wanted to. Considering the large January release in front of us and the PR to wrap up MUI migrations I would leave it alone for now
coliu-akamai
requested review from
mjac0bs,
carrillo-erik and
abailly-akamai
and removed request for
a team
|
Everything looks good to me. I had a hiccup running the test for
|
|
thanks @carrillo-erik! |
coliu-akamai
added
Add'tl Approval Needed
There was a problem hiding this comment.
Looks good! Thanks @coliu-akamai
Only thing I noticed is the Radio size prop not doing anything. Approving tho we may wanna look into that.
Also, don't hesitate to get a little deeper with testing props sometimes. Especially the ones that are going to change the appearance of the component. This is usually the kind of visual regression we miss when doing a upgrade or migration.
| @@ -73,6 +73,10 @@ interface BaseProps { | |||
| * className to apply to the underlying TextField component | |||
| */ | |||
| className?: string; | |||
| /** | |||
There was a problem hiding this comment.
I think this could be done separately if we wanted to. Considering the large January release in front of us and the PR to wrap up MUI migrations I would leave it alone for now
|
@coliu-akamai also restarting your e2e suite since once of the run failed to complete the job |
|
@abailly-akamai looking into the size issue, seems like size as a prop just doesn't have any effect on the radio appearance at all, even though the svg fontsize changes 🤔 gonna look into this some more!
edit: even if I just use Radio from mui directly, I'm not seeing the size prop do anything?? 😭 I might just merge this as is then, and create a new ticket to look into this some more |
bnussman-akamai
added
Approved
|
update: Alban and I investigated, and the issue is due to |
Description 📝
How to test 🧪
Verification steps
As an Author I have considered 🤔
Check all that apply