Closed teradaian closed 3 years ago
vercel[bot]
commented
3 years ago This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/stephin007/cowin-vaccine-availablity-checker/HhmewK5mLfpstERojFBsEMkTqvDv
✅ Preview: https://cowin-vaccine-git-fork-teradaian-new-ui-ste-d6e057.vercel.app
stephin007
commented
3 years ago checking @teradaian , lot of changes to review please give me some time! Also seems like rightnow the site is not responsive , what are your plans for that?
stephin007
commented
3 years ago @teradaian honestly, this draft looks dope! ⭐ left some comments please look into that as well!
teradaian
commented
3 years ago checking @teradaian , lot of changes to review please give me some time! Also seems like rightnow the site is not responsive , what are your plans for that?
I'm trying to figure out what failed- hopefully just a silly oversight on my end as it was working when I committed.
teradaian
commented
3 years ago @teradaian honestly, this draft looks dope! ⭐ left some comments please look into that as well!
Thanks! I'll look into all the comments, and appreciate the feedback!
stephin007
commented
3 years ago Also @teradaian the colors in the vaccine card are not suitable for the darkmode! What are your thoughts?
Also FYI: please pull the latest changes from the NEW-UI branch to avoid conflicts
stephin007
commented
3 years ago Please fix the merge conflicts, @teradaian 🙏🏾
stephin007
commented
3 years ago @teradaian ihave merged your PR, we will be working on the CSS issue!
Appreciate all your efforts learnt a lot from you! looking forward to work on more issues with you!
Justinnn07
commented
3 years ago nice work @teradaian 💯
teradaian
commented
3 years ago the difference between the MUIProvider and themeprovider is that the MUIprovider is strictly dealing with the theme setting of MUI elements, while the styled components deals with everything else. Since they couldn't share a name on import, I gave them more specific names.
If you check the MUI docs, you can also make changes to what the "dark" mode consists of, to your liking. I used their default so far :)
On Fri, May 28, 2021 at 6:07 PM Stephin Reji @.***> wrote:
@.**** approved this pull request.
In src/App.js https://github.com/stephin007/Cowin-Vaccine-Availablity-Checker/pull/112#discussion_r641785531 :
Hey @teradaian https://github.com/teradaian could you please explain the difference between themeprovider and MUIThemeProvider? do they have some specific role?
In src/components/useDarkMode.js https://github.com/stephin007/Cowin-Vaccine-Availablity-Checker/pull/112#discussion_r641782093 :
+const setMode = mode => {
- window.localStorage.setItem('theme', mode)
- setTheme(mode) +};
+const toggleTheme = () => {
- if (theme === 'light') {
- setMode('dark');
- } else {
- setMode('light');
- } +};
+useEffect(() => {
- const localTheme = window.localStorage.getItem('theme');
- if (localTheme) {
i guess its defaulted to light right? please correct me if i am wrong!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stephin007/Cowin-Vaccine-Availablity-Checker/pull/112#pullrequestreview-671599470, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARGE4PBKLVALVFGHZOUVXVDTQA46DANCNFSM45XACYEQ .
Feature: Adds basic scalable light/dark mode functionality using a wrapping Styled Component themeprovider + a MUI theme provider.
The useDarkTheme custom hook handles the styled component wrapper, which the MUI theme pairs with. All MUI features should work with dark mode now - if not let me know.
I included a Styled Component button for the toggle, but you could of course just make this a normal react feature and limit Styled Component integration to globalStyles only. globalStyles.js atm is using mostly simple selectors, but as things scale you can increase specificity ofc.
Let me know your thoughts on this second draft, thank you :)