se701g2 / Doto

Todo application for Group 2
https://doto.azurewebsites.net/
MIT License
12 stars 42 forks source link

bug/252-Fix checkbox not updating by abstracting state away from component renders closes #252 #254

Closed preetpatel closed 4 years ago

preetpatel commented 4 years ago
preetpatel commented 4 years ago

It crosses out the task on the checklist and the task is marked as complete which is good. However, the actual checkbox doesn't get ticked. If there's no way to get the checkbox to be ticked through code then I guess it's alright since the actual task is marked as complete, but it's just a cosmetic problem.

It did the tick for me when i ran it.. thats weird

preetpatel commented 4 years ago

Hi @PreetPatel ,

Thanks for working on this bug. I think that I might have found a new bug while checking this code out locally. When first accessing Doto I get the following:

  • After the authentication the following exception is thrown, "Unhandled Rejection (Type Error): Cannot read property 'themePreference". Although, there is a button to cross it out and proceed.
  • This then loads the home page, when trying to create a new task the following is thrown: "Cannot read property 'forEach"

I have attached screenshots below for reference (To document with the bug). image image

NOTE: After doing a refresh everything works as supposed to and I do not get the issue @EricPedrido mentioned. Thanks for fix @PreetPatel 👍. Thus, since everything does seem to work I have gone ahead and approved the PR and will make a bug report for what I encountered, and hopefully, we can triangulate it soon.

Cheers thanks @harmanlamba I think the issue you're having is due to not properly going through the auth flow for google

EricPedrido commented 4 years ago

I do not get the issue @EricPedrido mentioned

Alright, well I assume it's a problem on my end then, I'll remove my change request but I won't be approving it just in case someone else gets this issue too. Nice work though, everything else looks good.

jordansimsmith commented 4 years ago

Hey @PreetPatel, can you elaborate on why this is a fix? It looks functionally equivalent to me.

HarrisonLeach1 commented 4 years ago

Hey @PreetPatel, can you elaborate on why this is a fix? It looks functionally equivalent to me.

I agree, the behaviour from #252 can still be replicated. The checkbox should work without having to refresh the page.

jordansimsmith commented 4 years ago

Closing for now, feel free to reopen when its ready to be merged in :)

preetpatel commented 4 years ago

Rip.. just started working on it haha