Converted Main.js to a functional component #365
Conversation
|
Someone is attempting to deploy a commit to a Personal Account owned by @saadpasta on Vercel. @saadpasta first needs to authorize it. |
|
@ashutosh1297 can you address the requested changes and then rebase #366 to reflect these changes. |
|
Ah sorry, my bad! I had typed the response but forgot to submit review. Please check my response and let me know how to proceed. Thanks! @kartikcho |
|
It's all good @ashutosh1297, I've added a new comment with a more optimal solution than the one we have been using. This should also slightly improve performance of our app. |
|
@kartikcho I moved the whole localStorage logic to a custom hook, checkout the new commit for updates. Keeping the context API to share theme state through the component tree seems like a better idea than using localStorage calls everywhere, can you elaborate on how you were planning of getting rid of the context API? PS - sorry about the delay in replies, I am a little caught up with work but I will try and wrap this up soon |
There was a problem hiding this comment.
No worries, thanks for the update!
In hindsight, I thought we could move away from Context altogether and check for the localStorage key where ever we need the dark mode flag. Thinking about it now, it might cause needless overhead.
Lets scratch that idea and stick with Context calls.
|
Hey @ashutosh1297, could you run prettier on this branch to fix the checks? |
|
It's very bad that prettier doesn't show the file diff. Let me see if I can fix it. |
Can we set up pre-commit https://prettier.io/docs/en/precommit.html#option-3-pre-commithttpsgithubcompre-commitpre-commit? I usually use it other python project it works and I can setup Github Actions to run that and output the diff. Tbh, I can't find a better idea looking at the docs or there is a bot which will automatically commit if there are file diff https://pre-commit.ci/ |
|
I have used Husky in the past for adding a pre commit hook but it's a hit or miss honestly, it only works correctly for some git users. |
Should be simple enough that @saadpasta should install a Github App as instructed there. I will make a PR for adding pre-commit config then :). |
|
@naveen521kk I have added pre-commit cli app |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/saadpasta/developer-folio/DHzwg4NyoSvsfHjEsFD574PwuQxH |
Thanks, I will set up pre-commit in another PR. |
|
pre-commit.ci autofix |
|
@naveen521kk shouldn't this just run the prettier command in a new commit on this PR? So why is it failing? |
|
pre-commit.ci autofix |
|
Hmm, this looks confusing. |
@kartikcho I am not sure why it's failing, I ran the prettier script from package.json and it showed that everything's fixed. Are there more steps involved? |
Can you rebase with the master and then run |
|
I'll probably take a look at the tests this weekend but for now, try doing what Naveen suggested. |
|
@kartikcho are you looking into this. |
|
Sorry, I've been busy and this totally skipped my mind. Will try to take a look whenever possible. |
|
Hey @ashutosh1297, rebase from master and this should be green. |
|
Hey @ashutosh1297 are you still working on this? |
* Converted Main.js to a functional component * Moved localStorage theme logic into a custom hook * Add extra line to EOF * Prettier check fixes * fix formatting, rebase branch Co-authored-by: Ashutosh Sharma <ashutosh@thefastway.in> Co-authored-by: Kartik Choudhary <kartikc.918@gmail.com>
Pull request in resolution of issue #363