joshghent / gifbar

🦄 Find Awesome Gif's right in your Menu Bar
MIT License
42 stars 31 forks source link

migrate to Functional Components #159

Closed jamescallumyoung closed 3 years ago

jamescallumyoung commented 3 years ago

Modern React development has moved away from class-based components to functional components and hooks. This new approach allows complex behaviour to be modelled without the need for verbose event handling.

Since the components in this repo are rather small, I think it would be possible to re-write most of the existing components as Functional Components with little effort.

Modernising the codebase will allow for easier open-source contributions as more developers will be familiar with the format and Functional components are easier to learn for non-React developers.

jamescallumyoung commented 3 years ago

Hey @joshghent, it's been a while! Hope all has gone well for you in 2020!

I spotted this repo in my github feed earlier today and had a gander at the changes since I was last here.

I thought this might be a nice improvement that wouldn't take too much effort. Let me know what you think.

jamescallumyoung commented 3 years ago

Looks like someone already tried this way back in Jan with #129 but it was never merged.

I would suggest rejecting #129 though because:

14h commented 3 years ago

@jamescallumyoung I honestly just completely forgot about it... I will take another go at it

jamescallumyoung commented 3 years ago

Hey @14h. If you want to take another crack at it, great 👍 If not, I'm happy to do some, or we can add a help wanted tag and see who shows up. It's also probably a good first issue.

I'd recommend starting a new branch from master and closing #129 but if you think you can rescue your old PR then feel free. If you do start from 129, please move the eslint changes to another PR; I can make an issue for that if needed.

Also, please assign yourself when you start on a solution.

joshghent commented 3 years ago

@14h @jamescallumyoung This would be an awesome change so please let me know if you need any help. This project needs a lot of love to get back on it's feet because I merged a lot of PR's that I didn't check thoroughly enough... Could be a great little app tho

jamescallumyoung commented 3 years ago

@14h @jamescallumyoung This would be an awesome change so please let me know if you need any help. I'll let @14h pick this up and I'll also be here to help if needed. Feel free to (re)request a review as and when desired @14h 😃

Could be a great little app tho I totally agree! I actually use it quite often.

Additionally, I think projects this size/scope make great "first time" projects for new open-source contributors. With Hacktoberfest coming up, I though getting things ready would be a good idea. Some recent activity might help new contibutors feel more comfortable 👍

joshghent commented 3 years ago

@jamescallumyoung Completely agree! Let's try to get some issues going ready for new contributors 👍 Maybe we can fix up the test/deployment process and the security issues so it's easier for new PR's

felix-chin commented 3 years ago

I'm interested in working on this issue if you still need help.

jamescallumyoung commented 3 years ago

Hey @felix-chin, thanks for the interest. I've not seen any movement from @14h on this so it may be up fro grabs. I'd ask that you wait until this time tomorrow to see if this issue is free to pick up.

@14h, if you still want to work on this, could you reply here? 😄

felix-chin commented 3 years ago

Hey @jamescallumyoung, following up on this. Thanks

jamescallumyoung commented 3 years ago

Have at it @felix-chin! I'll assign it to you now 👍 Thanks for coming back to this.

Please tag me as a Reviewer in the PR whenever you open one. I'll take a look as soon as I can 👍