feat(ui): Add PhoneInput component with international formatting support #309
Conversation
…f, displayName); update all elements
|
@adityajha2005 is attempting to deploy a commit to the Hycom Team Team on Vercel. A member of the Team first needs to authorize it. |
|
👀 We've notified the reward creators here. |
| "baseUrl": "./", | ||
| "jsx": "preserve", | ||
| "resolveJsonModule": true, | ||
| "types": ["react", "react-dom"], |
There was a problem hiding this comment.
Are these changes really necessary? This config extends ui.json where e.g. jsx:preserver is already declared; did you add these here to solve some issue?
| @@ -0,0 +1,4 @@ | |||
| export { default as PhoneInput } from '../../elements/phone-input'; | |||
| export type { PhoneInputProps, PhoneInputChangePayload } from '../../elements/phone-input'; | |||
There was a problem hiding this comment.
placing this in the components folder is not necessary - this is still a rather simple component, and, like a regular Input, it's enough to have this one in elements as well
|
|
||
| const NAME = 'PhoneInput'; | ||
|
|
||
| export const PhoneInput = forwardRef<HTMLInputElement, PhoneInputProps>( |
There was a problem hiding this comment.
please update this component to not rely on forwardRef - you've already done that for other components in the other PR, so let's keep things consistent :)
| }} | ||
| type="tel" | ||
| inputMode="tel" | ||
| name={props.name ?? NAME} |
There was a problem hiding this comment.
NAME here is not needed - in the example code provided it was for adding data-test attributes for e2e testing, but we don't have that here (yet)
|
|
||
| const NAME = 'PhoneInput'; | ||
|
|
||
| export const PhoneInput = forwardRef<HTMLInputElement, PhoneInputProps>( |
There was a problem hiding this comment.
please add a Storybook story for this component; once that is prepared I'll take a look how it works
|
|
||
| import InputFormat from 'input-format/react'; | ||
| import { AsYouType, CountryCode, getCountryCallingCode, parsePhoneNumberCharacter } from 'libphonenumber-js/core'; | ||
| import type { MetadataJson } from 'libphonenumber-js/types'; |
There was a problem hiding this comment.
eslint reports here Cannot find module 'libphonenumber-js/types' or its corresponding type declarations - see if it can be imported in another way (perhaps in newer version exports have changed)
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore input-format expects a different component type | ||
| inputComponent={Input} | ||
| ref={ref} |
There was a problem hiding this comment.
TS reports here Property 'ref' does not exist on type 'IntrinsicAttributes & DefaultInputComponentProps & { parse: ParseFunction; format: FormatFunction; ... 5 more ...; onKeyDown?(value?: string | undefined): void; }' - please check how this can be fixed
| useEffect(() => { | ||
| if (value === undefined) return; // uncontrolled external value not provided | ||
| const formatted = format(value).text; | ||
| setText(formatted); |
There was a problem hiding this comment.
my IDE reports here an ESLint warning Error: Calling setState synchronously within an effect can trigger cascading renders - please see how this can be fixed
| const formatted = format(value).text; | ||
| setText(formatted); | ||
| // emit change based on incoming value as well | ||
| const asYouType = new AsYouType(undefined, metadata as any); |
There was a problem hiding this comment.
please see if/how casting as any can be removed from metadata as any and other places in this component; we'd like to have everything properly typed
|
hi @adityajha2005, are you still interested in contributing to this PR and bounty? |
|
hey @marcinkrasowski really sorry for the delay, yeah I will be working on this issue. |
|
@adityajha2005 no worries, take your time, I just wanted to know if you are still working on this |
2 participants
PR Description:
What does this PR do?
Related Ticket(s)
Key Changes
PhoneInputcomponent inpackages/ui/src/elements/phone-input.tsxthat extends the existing Input component for design system consistencylibphonenumber-js@^1.11.18andinput-format@^0.3.14to handle phone number parsing and real-time formattingpackages/ui/src/lib/phone/metadata.json) containing only EN, DE, and PL country data to minimize bundle size instead of importing full libphonenumber-js metadatapackages/ui/src/components/PhoneInput/index.tsmaking the component available as@o2s/ui/components/PhoneInputHow to test
package.jsonnpm installin the root directory to install new dependenciesStep-by-step testing:
Import the PhoneInput component:
Test basic functionality:
Test different scenarios:
500500500withdefaultCountry="PL"- should format to+48 500 500 500+49123456789- should format according to German rules0048123456789- should convert00to+and format properlyaria-invalidand returnisValid: falsedefaultCountry="DE"anddefaultCountry="GB"Test accessibility:
aria-invalidattribute is set for invalid inputsinputMode="tel"andtype="tel"are appliedFramework Experience Feedback
Working with this framework was quite pleasant overall:
Getting Started:
Development Experience:
package.jsonwas well thought outChallenges:
Overall: The framework strikes a good balance between flexibility and structure. The monorepo setup makes it easy to work across packages, and the existing patterns provide good guidance for new components. Having a reference implementation to build upon was extremely valuable and accelerated development..
Media (Loom or gif)
/claim #306