Add AddressInput component #41

naz3eh · 2021-11-30T20:26:12Z

This PR fixes #9
I've added the AddressInput component that returns the address for the entered ENS. This PR is still in development.
Everyone's thoughts are welcome.

Dhaiwat10 · 2021-12-02T14:17:12Z

@allcontributors add @Nazeeh21 for code

allcontributors · 2021-12-02T14:17:21Z

@Dhaiwat10

I've put up a pull request to add @Nazeeh21! 🎉

Dhaiwat10 · 2021-12-02T14:17:24Z

@allcontributors add @Dhaiwat10 for code, review

allcontributors · 2021-12-02T14:17:32Z

@Dhaiwat10

I've put up a pull request to add @Dhaiwat10! 🎉

Dhaiwat10

Great work overall! Left some comments and questions. Also, please add docstrings etc. We want to document our code well.

Dhaiwat10 · 2021-12-03T00:39:46Z

packages/components/.babelrc

-    "@babel/preset-react"
-  ]
+  "presets": ["@babel/preset-env", "@babel/preset-typescript", "@babel/preset-react"],
+  "plugins": ["@babel/plugin-proposal-class-properties"]


What was this added for?

While running the test, I was getting this error Support for the experimental syntax 'jsx' isn't currently enabled.

So I had a look at different issues and found that this was due to the babel_configurations. So I added this.

Dhaiwat10 · 2021-12-03T00:40:16Z

packages/components/babel.config.js

+module.exports = function (api) {
+  const presets = ['@babel/preset-env', '@babel/preset-react', '@babel/preset-flow'];
+  const plugins = ['@babel/plugin-transform-runtime'];
+
+  /** this is just for minimal working purposes,
+   * for testing larger applications it is
+   * advisable to cache the transpiled modules in
+   * node_modules/.bin/.cache/@babel/register* */
+  api.cache(false);
+
+  return {
+    presets,
+    plugins,
+  };
+};


Same as above. Why was this added?

This was also added to fix the above-mentioned error.

Dhaiwat10 · 2021-12-03T00:40:47Z

packages/components/src/components/AddressInput/AddressInput.stories.tsx

+import { ethers } from 'ethers';
+
+storiesOf('AddressInput', module).add('default', () => {
+  const provider = new ethers.providers.Web3Provider(window.ethereum);


Use useWallet in stories instead

Sure, I'll update that

Dhaiwat10 · 2021-12-03T00:41:23Z

packages/components/src/components/AddressInput/AddressInput.tsx

+export interface AddressInputProps {
+  provider: ethers.providers.Web3Provider;
+  value: string;
+  setValue: (value: string) => void;


Let's change this prop's name to onChange.

Dhaiwat10 · 2021-12-03T00:42:04Z

packages/components/src/components/AddressInput/AddressInput.tsx

+    } catch (error) {
+      console.log('error in fetching balance', error);
+    }


We need some error handling in the UI, too. At the moment, the user will not know if an error has occurred.

Got it. I'll add the alert, display it in the UI and also return an error message to the user.

How does this sound?

Dhaiwat10 · 2021-12-03T00:42:18Z

packages/components/src/components/AddressInput/AddressInput.tsx

+  };
+
+  useEffect(() => {
+    if (typeof window.ethereum !== 'undefined') {


Why do we need to check for this?

Ahh, I forgot to remove it 😅 . While developing, I was using it but I am no longer using it in any of the places. I'll remove it.

Dhaiwat10 · 2021-12-03T00:50:07Z

packages/components/src/components/AddressInput/AddressInput.tsx

+  }, [inputValue]);
+  return (
+    <div>
+      <Text>Input address</Text>


This is a label so use <FormLabel /> from Chakra instead

Dhaiwat10 · 2021-12-03T00:50:22Z

packages/components/src/components/AddressInput/AddressInput.tsx

+    }
+  }, [inputValue]);
+  return (
+    <div>


We don't need this div

Dhaiwat10 · 2021-12-03T01:33:04Z

packages/components/src/components/AddressInput/AddressInput.tsx

+      <Input
+        mt={2}
+        mb={2}
+        value={inputValue}
+        onChange={(e) => setInputValue(e.target.value)}
+        placeholder='Input address'
+      />


We want to support all props that a Chakra <Input /> supports and spread them here onto this <Input />. Let me know if you need help with this 😄

Sure! I'll try implementing it and will surely ask you for help if I'll get stuck somewhere.

naz3eh · 2021-12-04T11:40:55Z

I've updated the PR implementing the requested changes.
Now, I'm looking at the test to get it passed.

Dhaiwat10 · 2021-12-06T00:13:10Z

@Nazeeh21 can you please post more information about what problems you are facing with the tests? I am asking around for help and having some tldr info on the problem would be great.

I ran the test locally and it seems like it keeps on running forever. For anyone who's reviewing, please run yarn in the root and then yarn test -t="AddressInput"

narbs91 · 2021-12-06T02:43:26Z

packages/components/src/components/AddressInput/AddressInput.tsx

+  const getAddressFromEns = async () => {
+    try {
+      let address = await provider.resolveName(deboucedValued);
+      console.log({ address });


[nit pick] did you want to leave the console log statement here?

No, it's solely for dev purposes. I'll remove it.

narbs91 · 2021-12-06T02:48:20Z

packages/components/src/components/AddressInput/AddressInput.tsx

+}) => {
+  const [inputValue, setInputValue] = useState('');
+  const deboucedValued = useDebounce(inputValue, 700);
+  const [, setEnsValue] = useState('');


Do we need this state setter here? Since we're not reading the value, do we really need to set it?

Yeah, the use of this variable can be avoided, and probably it won't be needed.

narbs91 · 2021-12-06T02:54:44Z

packages/components/src/components/AddressInput/AddressInput.test.tsx

+
+describe('AddressInput', () => {
+  it('renders', () => {
+    const provider = new MockProvider();


This is the repo that @codingwithmanny came up with when we were exploring mocking providers. In the link you'll find how the metamask provider was mocked and you can use that as a guide; however, there doesn't seem to be a universal mock wallet provider as I think everyone kinda has there own implemented standards

Thanks for this reference. I'll take a look.

Thanks for the review @narbs91!

No prob at all, this things looking pretty dope you've all done an awesome job! Keep it up 🚀

This is the repo that @codingwithmanny came up with when we were exploring mocking providers.

@narbs91 This worked for me and tests are now passing. Thanks for helping me out. 😃

naz3eh · 2021-12-06T04:57:37Z

can you please post more information about what problems you are facing with the tests? I am asking around for help and having some tldr info on the problem would be great.

I ran the test locally and it seems like it keeps on running forever. For anyone who's reviewing, please run yarn in the root and then yarn test -t="AddressInput"

Yeah sure. For me also the tests were not giving any output while running yarn test -t="AddressInput" in the root directory. However, when I run npx jest ./src/components/AddressInput in /packages/components/ folder, I came across this error.

I am trying the solution provided by @narbs91, hopefully, that would fix the error.

Dhaiwat10 · 2021-12-06T05:21:08Z

Awesome @Nazeeh21, let us know if you find a fix

naz3eh marked this pull request as draft November 30, 2021 20:26

Dhaiwat10 self-requested a review December 2, 2021 00:48

allcontributors bot mentioned this pull request Dec 2, 2021

docs: add Nazeeh21 as a contributor for code #52

Merged

allcontributors bot mentioned this pull request Dec 2, 2021

docs: add Dhaiwat10 as a contributor for code, review #53

Merged

Dhaiwat10 requested changes Dec 3, 2021

View reviewed changes

naz3eh added 5 commits December 4, 2021 13:04

Add AddressInput component

a4baa4d

Add MockProvider

72ce180

Add debounce

9e08bb9

Add FormControl, Error handling and chakra props to AddressInput

69b4c9a

Use useWalletProvider in storybook

d1fa56d

naz3eh force-pushed the fix/issue-9 branch from 90595e6 to d1fa56d Compare December 4, 2021 10:31

narbs91 reviewed Dec 6, 2021

View reviewed changes

naz3eh added 2 commits December 6, 2021 11:21

refactor AddressInput

6252835

Fix debouncedValue typo

65fd656

naz3eh marked this pull request as ready for review December 6, 2021 06:14

Pass AddressInput test

ee9601a

naz3eh force-pushed the fix/issue-9 branch from 4277e29 to ee9601a Compare December 6, 2021 06:25

naz3eh and others added 3 commits December 6, 2021 12:08

Add test to check if value changes correctly

5b8363f

Update test name

534b042

Add label prop, remove hard defaults

50fe0e0

Dhaiwat10 merged commit ed78267 into master Dec 6, 2021

Dhaiwat10 deleted the fix/issue-9 branch December 6, 2021 07:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AddressInput component #41

Add AddressInput component #41

naz3eh commented Nov 30, 2021 •

edited

Loading

Dhaiwat10 commented Dec 2, 2021

allcontributors bot commented Dec 2, 2021

Dhaiwat10 commented Dec 2, 2021

allcontributors bot commented Dec 2, 2021

Dhaiwat10 left a comment

Dhaiwat10 Dec 3, 2021

naz3eh Dec 3, 2021 •

edited

Loading

Dhaiwat10 Dec 3, 2021

naz3eh Dec 3, 2021

Dhaiwat10 Dec 3, 2021

naz3eh Dec 3, 2021

Dhaiwat10 Dec 3, 2021

Dhaiwat10 Dec 3, 2021

naz3eh Dec 3, 2021

Dhaiwat10 Dec 3, 2021

naz3eh Dec 3, 2021

Dhaiwat10 Dec 3, 2021

Dhaiwat10 Dec 3, 2021

Dhaiwat10 Dec 3, 2021

naz3eh Dec 3, 2021

naz3eh commented Dec 4, 2021 •

edited

Loading

Dhaiwat10 commented Dec 6, 2021

narbs91 Dec 6, 2021

naz3eh Dec 6, 2021

narbs91 Dec 6, 2021

naz3eh Dec 6, 2021

narbs91 Dec 6, 2021

naz3eh Dec 6, 2021

Dhaiwat10 Dec 6, 2021

narbs91 Dec 6, 2021

naz3eh Dec 6, 2021

naz3eh commented Dec 6, 2021

Dhaiwat10 commented Dec 6, 2021

Add AddressInput component #41

Add AddressInput component #41

Conversation

naz3eh commented Nov 30, 2021 • edited Loading

Dhaiwat10 commented Dec 2, 2021

allcontributors bot commented Dec 2, 2021

Dhaiwat10 commented Dec 2, 2021

allcontributors bot commented Dec 2, 2021

Dhaiwat10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naz3eh Dec 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naz3eh commented Dec 4, 2021 • edited Loading

Dhaiwat10 commented Dec 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naz3eh commented Dec 6, 2021

Dhaiwat10 commented Dec 6, 2021

naz3eh commented Nov 30, 2021 •

edited

Loading

naz3eh Dec 3, 2021 •

edited

Loading

naz3eh commented Dec 4, 2021 •

edited

Loading