The Wayback Machine - https://web.archive.org/web/20200615011414/https://github.com/ifmeorg/ifme/issues/1656
Skip to content

/ ifme

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

Jump to bottom

Re-write existing React components with new Hooks API #1656

Open
julianguyen opened this issue Feb 8, 2020 · 74 comments
Open

Re-write existing React components with new Hooks API #1656

julianguyen opened this issue Feb 8, 2020 · 74 comments
Assignees
@Cijin @wstan2 @gdemu13 @alekpentchev @KenAustria @Orelongz @thesid01 @isabia @Manasa2850 @RomanMaru
Labels
beginner friendly react refactoring

Comments

@julianguyen
Copy link
Member

@julianguyen julianguyen commented Feb 8, 2020 •
edited by Cijin

Description

We're currently on React 16.9.0, which means we can leverage the Hooks API. Let's re-write our components to use it where suitable.

I'm going to go ahead and divide the work so you can assign yourselves to it:

  • client/app/components/Accordion/index.jsx [@RomanMaru]
  • client/app/components/BaseContainer/index.jsx [@thesid01]
  • client/app/components/Chart/ChartControl.jsx [@Cijin]
  • client/app/components/Form/DynamicForm.jsx AND client/app/components/Form/index.jsx [@alekpentchev]
  • client/app/components/Header/index.jsx [@alekpentchev]
  • client/app/components/Input/index.jsx AND client/app/components/Input/InputCheckboxGroup.jsx AND client/app/components/Input/InputLocation.jsx AND client/app/components/Input/InputSelect.jsx AND client/app/components/Input/InputSwitch.jsx AND client/app/components/Input/InputTag.jsx [@Orelongz]
  • client/app/components/Modal/index.jsx [@alekpentchev]
  • client/app/widgets/Comments/index.jsx [@gdemu13]
  • client/app/widgets/Notifications/index.jsx [@kelsyvghn]
  • client/app/widgets/QuickCreate/index.jsx [@KenAustria]
  • client/app/widgets/Resources/index.jsx [@gdemu13]

If there are any missing tests, please add them 🙏

Please comment below and let me know which files you want to tackle. I'll update this comment accordingly!

Thanks everyone 🙌

Please assign yourself (via the Assignees dropdown), if you do want to work on this issue. Can't find yourself? You need to join our organization.

Check out our Picking Up Issues guide if you haven't already!
@wstan2
Copy link

@wstan2 wstan2 commented Feb 8, 2020

Can I help with this?
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented Feb 8, 2020

@wstan2 Of course! Feel free to assign yourself to this issue :D Thanks!
@Loriick Loriick self-assigned this Feb 14, 2020
@Loriick
Copy link

@Loriick Loriick commented Feb 14, 2020

hello, I assign myself to this issue , I hope it's okay ?
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented Feb 14, 2020

Go head! Actually I think @Loriick and @wstan2 could split up the work! I recommend deciding that in this issue :)
@RomanMaru
Copy link
Contributor

@RomanMaru RomanMaru commented Feb 17, 2020

Hello there, I'd like to help with this
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented Feb 19, 2020

Hey I think you all @Loriick @wstan2 and @RomanMaru should sync on this! Figure out how you want to split up the work! :)
@alekpentchev
Copy link
Contributor

@alekpentchev alekpentchev commented Feb 25, 2020

Hi all :)
I'd love to help with some components if there's something left. Let me know how it goes :)
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented Feb 26, 2020 •
edited

Hey folks who expressed interest in this @Loriick @wstan2 @RomanMaru @alekpentchev:

(See PR description)
@RomanMaru
Copy link
Contributor

@RomanMaru RomanMaru commented Feb 26, 2020

@wstan2
Copy link

@wstan2 wstan2 commented Feb 26, 2020

Can I take the last 3 components? Thanks.
@Orelongz
Copy link
Contributor

@Orelongz Orelongz commented Feb 26, 2020

I would like to take the Input component. Thanks
@alekpentchev
Copy link
Contributor

@alekpentchev alekpentchev commented Feb 27, 2020

Good idea! I'm ok with the remaining components:

  • client/app/components/Form/DynamicForm.jsx AND client/app/components/Form/index.jsx
  • client/app/components/Header/index.jsx
  • client/app/components/Modal/index.jsx
  • client/app/widgets/Comments/index.jsx
@alekpentchev
Copy link
Contributor

@alekpentchev alekpentchev commented Feb 29, 2020

How we should approach testing functional component's state?
I was doing a research on that topic and, as far as I know, there aren't any good methods of doing this (like with good ol' wrapper.state()). Testing state is even discouraged in many resources.
I've rewritten DynamicForm for now and all tests rely on state values so I'm wondering how to tackle this.
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented Mar 1, 2020

Good question @alekpentchev!

So we shouldn't be testing state objects directly. We should be testing for its effect on the UI.
@alekpentchev
Copy link
Contributor

@alekpentchev alekpentchev commented Mar 1, 2020

@julianguyen I strongly agree with you. Since all tests in DynamicForm and Form are relying on component's internal state I'll try to rewrite them so that they refer to the UI effect.
@alekpentchev alekpentchev mentioned this issue Mar 5, 2020
Closed
@Loriick Loriick removed their assignment Mar 5, 2020
This was referenced Mar 6, 2020
Merged
Merged
@manu29d
Copy link

@manu29d manu29d commented Mar 13, 2020

Hi @julianguyen,
Is this issue still under development? I see some PRs merged 7 days ago but I am not sure about the rest. If yes, could you update the checklist above please.
Thanks.
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented Mar 14, 2020

@manu29d is still under development! The checklist above should be updated :) I'm not sure how far people are with the files they are working on. You may be able to pick up some work here if folks want to split things up more.
@alekpentchev
Copy link
Contributor

@alekpentchev alekpentchev commented Mar 14, 2020 •
edited

I’ve done 2/4 components already and I’m slowly finishing the 3rd one. So in my case there isn’t much sense to split up the work. But I’ll let you know if I’d need support :)
@gdemu13 gdemu13 mentioned this issue May 9, 2020
Merged
@gdemu13
Copy link
Contributor

@gdemu13 gdemu13 commented May 9, 2020

Hi @julianguyen, if client/app/widgets/Resources/index.jsx is still available I can work on it
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented May 9, 2020

@gdemu13 Feel free to take it! Thanks so much :D
@gdemu13 gdemu13 mentioned this issue May 10, 2020
Merged
@Manasa2850
Copy link
Contributor

@Manasa2850 Manasa2850 commented May 11, 2020

@julianguyen I'm not working on client/app/widgets/Notifications/index.jsx currently. So anyone who wants can work on it. You can refer to my PR if needed.
@kelsyvghn
Copy link

@kelsyvghn kelsyvghn commented May 19, 2020

Hi I'd love to help if there's any work that still needs doing?
@Cijin Cijin self-assigned this May 22, 2020
@Cijin
Copy link

@Cijin Cijin commented May 23, 2020

Can i work on this

client/app/widgets/QuickCreate/index.jsx
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented May 23, 2020

Hey @kelsyvghn feel free to work on client/app/widgets/Notifications/index.jsx

@Cijin feel free to work on client/app/components/Chart/ChartControl.jsx.
@thesid01
Copy link
Member

@thesid01 thesid01 commented May 24, 2020

Hi would love to help if there's something i can work on !!
@seannemann21
Copy link

@seannemann21 seannemann21 commented May 24, 2020

Hello, @cmcWebCode40 are you still working on client/app/components/BaseContainer/index.jsx? I noticed the PR was closed, so I was just checking in.
@cmcWebCode40
Copy link

@cmcWebCode40 cmcWebCode40 commented May 25, 2020

@seanneman21 I couldn't continue because I'm having issues setting up the working and running the app with my system ,my system is an old it couldnt carry it.
You can assign someone else to do the work. Thanks
@seannemann21
Copy link

@seannemann21 seannemann21 commented May 25, 2020

@thesid01 Would you like to work on client/app/components/BaseContainer/index.jsx? If not, I would be willing to since I also was looking into working on something.
@thesid01
Copy link
Member

@thesid01 thesid01 commented May 25, 2020

@seannemann21 Yes I am taking up this issue client/app/components/BaseContainer/index.jsx?
@thesid01 thesid01 self-assigned this May 26, 2020
@Cijin
Copy link

@Cijin Cijin commented May 29, 2020

Refactored QuickCreate and pushed the branch. Is there something else I need to do after.
@thesid01
Copy link
Member

@thesid01 thesid01 commented May 31, 2020

I have recreated BaseContainer/index.jsx using hooks and created a pull request. Someone review it.
@midnightmarth
Copy link

@midnightmarth midnightmarth commented Jun 2, 2020

Are there still components that need to be rewritten? I want this to be my first repo to contribute to because of the nature of this project.
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented Jun 3, 2020

@Cijin Hey that's great, you need to make a pull request next. Let me know if you need any help. Here's our guide on it.
@julianguyen
Copy link
Member Author

@julianguyen julianguyen commented Jun 3, 2020

@midnightmarth Looks like ChartControl is up for grabs since @Cijin did QuickCreate instead of that. Feel free to take that on.
@Cijin
Copy link

@Cijin Cijin commented Jun 3, 2020

Thank you @julianguyen. Will do that.
@manvendra22
Copy link

@manvendra22 manvendra22 commented Jun 3, 2020

@julianguyen Hey I am also looking for my first-time contribution. Please let me know if there is anything available to work on.
@thesid01
Copy link
Member

@thesid01 thesid01 commented Jun 4, 2020

Hey @kelsyvghn are you working on client/app/widgets/Notifications/index.jsx ?
If not then I can work on it.
@kelsyvghn
Copy link

@kelsyvghn kelsyvghn commented Jun 4, 2020

@maegatro
Copy link

@maegatro maegatro commented Jun 5, 2020

Hello, I'd love to work on this, is there anything I can help?
@KenAustria
Copy link

@KenAustria KenAustria commented Jun 5, 2020

Hey @manvendra22, please take over 'client/app/widgets/QuickCreate/index.jsx' if you'd like. Otherwise, it's all yours @maegatro! Sorry, I haven't found the time.
@manvendra22
Copy link

@manvendra22 manvendra22 commented Jun 5, 2020

Sure @KenAustria I'll take it.Thanks
@kshashwat007
Copy link

@kshashwat007 kshashwat007 commented Jun 5, 2020

Hi, Wanted to know if there is anything i can work on? Would love to contribute!
@at-syot
Copy link

@at-syot at-syot commented Jun 5, 2020 •
edited

Hi, Any component i can help ?
Could i go for ChartControl.jsx ?
@ysun62
Copy link

@ysun62 ysun62 commented Jun 7, 2020

Hello all, is there anything left that I can contribute to?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner friendly react refactoring
You can’t perform that action at this time.