ccfp / github-issue-finder

GitHub issue finder
https://github-issue-finder.ccfp.now.sh
MIT License
3 stars 0 forks source link

Language select component always changes to "Any" #10

Closed pete-murphy closed 5 years ago

pete-murphy commented 5 years ago

The onChange handler for the Languages select element will change to "Any" regardless of what language is selected.

pete-murphy commented 5 years ago

Just realized that the Keywords input is broken as well, so this probably has to do with the useInput custom hook. Either need to refactor that function so it returns values in the array in a different order, or change where I'm calling it.

VinceGrilli commented 5 years ago

So I've been looking at these two problems for a little while now and I'm not coming up with much. This is my first time messing with custom hooks but I've been over the docs. And the code makes sense to me. I just can't see why our value isn't updating and its just printing an object prototype. I'm gonna spend the rest of the night researching custom hooks in react forms and give it another go tomorrow

pete-murphy commented 5 years ago

It doesn't help that my code so far is a bit of a mess 🙈, but I think I see the issue and might be able to guide you towards it. (If hooks are new then there's a bit of a learning curve there too, but I'm confident you'll get it.) Check out what's being returned from useInput compared to what I'm expecting it to return where I'm calling it, I think I just got the order confused (like using handle***Change where I should be using set***) if that makes sense. I'd be down to pair program with you tomorrow if you have Zoom set up.

pete-murphy commented 5 years ago

Or if (understandably) hesitant to use Zoom, Google Hangouts would work too.

VinceGrilli commented 5 years ago

I probably should have looked here first but I gave it another go after work. I broke the inputs down into separate components but that was probably not what we needed, and if its counter-productive don't pull the branch I just pushed. You we're right about changing the order, it got the keywords and language input to work but broke the label input. And the label input is the part I'm not 100% getting. Anyway I'll have some more time a little later and hopefully I can figure a little more out. just wanted to give an update

pete-murphy commented 5 years ago

Just reading over it on my phone, it’s looking good, and definitely would want to put in separate components, the only thing is now the form submit handler doesn’t have access to the input values (I don’t think, I should add tests for that 🙂) but that’s not a big deal because we’d hook those up to Redux eventually anyways, so as long as inputs are working independently should be fine.

Keep on it if you can and take your time, thought I’d have a chance to pair program but working late tonight twisting wires together trying not to shock myself too much ⚡️

pete-murphy commented 5 years ago

Realizing its probably unclear what I meant by “keep at it” but go ahead and make a PR to merge it to master if you want, I can review either later tonight or sometime tomorrow

VinceGrilli commented 5 years ago

👍 Ok great thanks man. I'll do a little more tonight and hopefully figure out that label input. And merge it when I'm done. Don't electrocute yourself!