Irregularities in the naming of variables made it confusing to know what a variable actually contains.
useContext hook was implemented incorrectly. The way it was implemented is a very common mistake so it was easy to catch.
Changes
I fixed the useContext implementation.
Removed the line which made the page refresh.
My Suggestion
Since the theme variable can only contain two values (light and dark), can we convert that into a boolean? That would make the logic much simpler.
In the context file, we are exporting a state variable and a setTheme variable which might cause confusion. I think converting the state to a boolean will solve all the confusion. The code seems too complex for such basic logic.
I did not fix the naming convention. I would like to discuss it before changing anything as it could cause confusion. 👍
Screenshot
Related PR
The original pull request #178 was deleted by mistake that contained some discussion, therefore this is the new PR.
Signed-off-by: Devesh deves125@gmail.com
Description
This bug was occurring because of a few reasons:
useContexthook was implemented incorrectly. The way it was implemented is a very common mistake so it was easy to catch.Changes
useContextimplementation.My Suggestion
Since the theme variable can only contain two values (
lightanddark), can we convert that into a boolean? That would make the logic much simpler.In the context file, we are exporting a
statevariable and asetThemevariable which might cause confusion. I think converting the state to a boolean will solve all the confusion. The code seems too complex for such basic logic.I did not fix the naming convention. I would like to discuss it before changing anything as it could cause confusion. 👍
Screenshot
Related PR
The original pull request #178 was deleted by mistake that contained some discussion, therefore this is the new PR.
How to test
You can simply toggle the theme change button
fixes #165