-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add Form section pattern #334
Conversation
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.
Hey @jaronheard is there a way to view this form? Is it in storybook?
@@ -7,20 +7,23 @@ import PasswordField from "../Fields/PasswordField"; | |||
const fields = { | |||
oldPassword: { | |||
label: "Old Password", | |||
section: "oldPassword", |
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.
Hey @jaronheard, this seems like a good format. Does section
need to be defined? is there a fallback if there is only one section?
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.
Section doesn't need to be defined in fields (though there's probably some ways to break it right now, need to add tests)
Form takes an optional sections prop array of sections, and returns form
and formSections
</React.Fragment> | ||
); | ||
|
||
const formSections = |
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.
Can you walk me through the formSection is doing here. I see that you are returning children(form, formSections) further down. I'm getting lost in how it's building together.
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.
basically --
iterate over each section in the sections
prop array (e.g. ["sectionOne","sectionTwo"]) and return a formSections object that looks like this:
{
sectionOne: { // all `fields` with section === `sectionOne`}
sectionTwo: { // all `fields` with section === `sectionTwo`}
}
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.
Ok. So we are expecting a consistent naming convention of sectionNUMBER
? Is it making sure sectionOne
comes before sectionTwo
, and sectionTwo
comes before sectionThree
?
@andrewbiang888 yes, the ChangePassword page in Storybook is using this. Though it doesn't look at all different than it did before. If you wrapped |
Sorry, knocked out that example pretty quickly. I think sections should be named with descriptive names — imagine what the section header would be and that should be the section name.
…On Fri, Jun 14, 2019 at 3:35 PM, Andrew < ***@***.*** > wrote:
***@***.**** commented on this pull request.
In app/ src/ components/ Forms/ Form. js (
#334 (comment)
) :
> </React.Fragment> ); - + const formSections =
Ok. So we are expecting a consistent naming convention of sectionNUMBER ?
Is it making sure sectionOne comes before sectionTwo , and sectionTwo comes
before sectionThree ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub (
#334?email_source=notifications&email_token=ABV5AX3IIUGCJGMGPD7BG2TP2QMMHA5CNFSM4HX63K6KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3UQMLY#discussion_r294013620
) , or mute the thread (
https://github.com/notifications/unsubscribe-auth/ABV5AX7A4BG54BCBKGC5KELP2QMMHANCNFSM4HX63K6A
).
|
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.
Great, great start.
This demonstrates a potential pattern for form sections. It also shows an example using ChangePassword.
If you pass a an array of section names as a prop to Form, then it will give you back a formSections object.
Like so:
Then in the form page, where you'd use
form
, instead you'd useformSections.userInfo
andformSections.passwordInfo
.Next steps are: