-
Notifications
You must be signed in to change notification settings - Fork 9
Add basic form fields #4961
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
base: feature/contact-form
Are you sure you want to change the base?
Add basic form fields #4961
Conversation
8b95146 to
ba6442b
Compare
ba6442b to
fb4c256
Compare
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.
The form will be implemented using https://react-hook-form.com/ later, does it make sense to implement components with local state now?
| @@ -0,0 +1,15 @@ | |||
| interface CheckboxFieldProps { | |||
| label: 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.
Please replace all namespace imports (React.*) with real imports.
| @@ -0,0 +1,15 @@ | |||
| interface CheckboxFieldProps { | |||
| label: React.ReactNode; | |||
| helperText?: string; | |||
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.
Please use ReactNode for this for better i18n. This allows using <FormattedMessage> instead of intl.formatMessage().
| required?: boolean; | ||
| placeholder?: string; | ||
| helperText?: string; | ||
| textArea?: boolean; |
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.
Should we instead create a TextField and a TextareaField?
| label: React.ReactNode; | ||
| required?: boolean; | ||
| helperText?: string; | ||
| options: Array<{ value: string; label: string }>; |
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.
Please use ReactNode for the label.
| required = false, | ||
| helperText, | ||
| options, | ||
| placeholder = "Select an option", |
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.
The default value should be a react-intl message.
| required?: boolean; | ||
| } | ||
|
|
||
| export function CheckboxField({ label, helperText }: CheckboxFieldProps): React.ReactElement { |
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.
We can remove the explicit return type definitions here.
Description
ContactFormBlock(Styling and functionality will be added in upcoming PRs
Acceptance criteria
Screenshots/screencasts
Open TODOs/questions
ContactFormBlock#4959Further information