- Notifications
You must be signed in to change notification settings - Fork928
refactor: Add components for styling forms#1169
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedApr 25, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #1169 +/- ##==========================================- Coverage 66.53% 66.23% -0.30%========================================== Files 263 265 +2 Lines 16427 16593 +166 Branches 156 157 +1 ==========================================+ Hits 10929 10991 +62- Misses 4377 4462 +85- Partials 1121 1140 +19
Continue to review full report at Codecov.
|
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.
Nice work!
export interface FullPageFormProps { | ||
title: string | ||
detail?: React.ReactNode |
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.
Had to read up again to remind myself what exactly is considered as aReactNode
anything that is renderable inside of JSX, this is NOT the same as what can be rendered by a component!
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.
@jsjoeio do you think it's the wrong type? I took it fromFormTitle
which is where this ends up but I could change it.
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.
Oh no, sorry, I think it's right. It just surprised me, that's all. I thoughtdetail
might have been astring
when I first saw the component. Whatever you think is best!
* Add FormFooter* Add FullPageForm* Lint* Make detail optional* Use Language
I thought some reusable components would help us with our create/edit forms.
See stories for example usage - the
FormFooter
goes inside aform
which goes inside theFullPageForm
.