Added AddressInput Component #167
Conversation
…act-components into address-input-with-prefix
ESLint Summary View Full Report
Report generated by eslint-plus-action |
src/inputs/AddressInput/index.tsx
Outdated
|
|
||
| export default AddressInput; | ||
|
|
||
| const TextField = styled(TextFieldMui)` |
There was a problem hiding this comment.
I would define this on top or into a separate file. It's a long component and wasn't expecting to find it here
There was a problem hiding this comment.
Also, was there some issue why we didn't use the existing TextField in src/inputs/TextField/index
There was a problem hiding this comment.
Added a new TextFieldInput and deprecated the old TextField component because is coupled to Final Form
| // const StyledTextField = styled(TextFieldInput)` | ||
| // && { | ||
| // .MuiFilledInput-root { | ||
| // background-color: lightgreen; | ||
| // width: 200px; | ||
| // transition: width 1s ease-out; | ||
| // } | ||
|
|
||
| // .MuiFilledInput-root.Mui-focused { | ||
| // width: 400px; | ||
| // } | ||
|
|
||
| // .MuiFormLabel-root.Mui-focused { | ||
| // color: ${({ error, theme }) => | ||
| // error ? theme.colors.error : 'darkgreen'}; | ||
| // } | ||
|
|
||
| // .MuiInputLabel-filled { | ||
| // color: ${({ theme, error }) => (error ? theme.colors.error : 'purple')}; | ||
| // } | ||
|
|
||
| // .MuiFilledInput-underline:after { | ||
| // border-bottom: 2px solid | ||
| // ${({ theme, error }) => (error ? theme.colors.error : 'orange')}; | ||
| // } | ||
| // } | ||
| // `; |
There was a problem hiding this comment.
I added that comment to be able to see the styles in the storybook:
But now I think that its better add the code inside a <pre> tag:
src/utils/address.ts
Outdated
|
|
||
| // Based on https://docs.ens.domains/dapp-developer-guide/resolving-names | ||
| // [...] a correct integration of ENS treats any dot-separated name as a potential ENS name [...] | ||
| const isValidEnsName = (name: string): boolean => name.includes('.'); |
There was a problem hiding this comment.
This only checks that there's a dot but not any other symbols? . is not a valid ens name
There was a problem hiding this comment.
Good catch!! I will update it to match with something.somethig
What is your opinion about this regex?
const isENSRegex = new RegExp(/[^\[\]]+\.[^\[\]]/)
src/inputs/AddressInput/index.tsx
Outdated
| useEffect(() => { | ||
| if (isValidAddress(address) && !isChecksumAddress(address)) { | ||
| onChangeAddress(checksumAddress(address)); | ||
| } | ||
| }, [address, onChangeAddress]); | ||
|
|
||
| // ENS resolution | ||
| useEffect(() => { | ||
| const resolveDomainName = async (ENSName: string) => { | ||
| try { | ||
| setIsLoadingENSResolution(true); | ||
| const address = (await getAddressFromDomain?.(ENSName)) as string; | ||
| onChangeAddress(address); | ||
| setPrefix(showNetworkPrefix ? networkPrefix : ''); | ||
| } catch (e) { | ||
| onChangeAddress(ENSName); | ||
| setPrefix(''); | ||
| } finally { | ||
| setIsLoadingENSResolution(false); | ||
| } | ||
| }; | ||
|
|
||
| const isEnsName = isValidEnsName(address); | ||
|
|
||
| if (isEnsName && getAddressFromDomain) { | ||
| throttle(() => resolveDomainName(address)); | ||
| } | ||
| }, [ | ||
| address, | ||
| getAddressFromDomain, | ||
| networkPrefix, | ||
| showNetworkPrefix, | ||
| onChangeAddress, | ||
| throttle, | ||
| ]); |
There was a problem hiding this comment.
Can this be merged into a single effect? I'm not sure if it's a good idea to run multiple effects that, in the end, call the same onChangeAddress function, could be subject to potential race conditions and bugs.
There was a problem hiding this comment.
done! now we only have one useEffect!
each time we use onChangeAddress we checksumValidAddress first:
onChangeAddress(checksumValidAddress(address));
Added some Accessibility improvements in the AddressInput Component.