-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: add MultiInput component #693
base: master
Are you sure you want to change the base?
Conversation
Features
ContributorsCommit-Lint commandsYou can trigger Commit-Lint actions by commenting on this PR:
|
Visit the preview URL for this PR (updated for commit eba0521): https://rainbow-modules--pr693-feat-add-multi-input-oyhkkx9u.web.app (expires Wed, 12 Apr 2023 19:50:44 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: a8717bcb415e206776430df4c4296748001a9951 |
|
||
# Overview | ||
|
||
`MultiInput` is an component that allows you to enter many input. |
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.
let's have chat GPT to help with the documentation
import { Field, UniversalForm } from '@rainbow-modules/forms'; | ||
import MultiInput from '../../src/components/MultiInput'; | ||
|
||
const GOOGLE_MAPS_APIKEY = process.env.STORYBOOK_GOOGLE_MAPS_APIKEY; |
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 checked and we already have an env for this called STORYBOOK_GOOGLE_MAPS_API_KEY in circle CI then let's change it here
<Field | ||
name="phones" | ||
label="Phone numbers" | ||
component={(props) => <MultiInput component={PhoneInput} {...props} />} |
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.
let's change the prop name component
to inputComponent
then we can use it like
<Field
label="Phone numbers"
component={MultiInput}
inputComponent={PhoneInput}
validate={validate}
/>
@@ -0,0 +1,101 @@ | |||
import React from 'react'; | |||
import { mount, shallow } from 'enzyme'; |
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.
let's not use enzyme anymore, instead use react testing library
label?: ReactNode; | ||
error?: ReactNode; | ||
value?: T; | ||
onChange?: (value: any) => void; |
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 must avoid using any
? valueInProps | ||
: [[undefined, undefined]]; | ||
|
||
const updatePhone = (index: number, newValue: any) => { |
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.
updatePhone?
? valueInProps | ||
: [[undefined, undefined]]; | ||
|
||
const updatePhone = (index: number, newValue: any) => { |
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.
no any instead you can use unkown
: [{ label: isFirst ? 'Primary' : 'Secondary' }, { label: 'Note' }]; | ||
|
||
const rowError = !error || typeof error === 'string' ? null : (error[index] as RowError); | ||
const phoneError = rowError && rowError.value; |
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.
change var name phoneError
import { InputConfig, RowError } from './types'; | ||
|
||
interface InputProps<T> { | ||
label?: 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.
I think label is always required
No description provided.