-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Conversation
Installed next-themes package & wrapped the application with ThemesProvider
Used emojis to convey mode & added dark class to text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great having you contribute to this project
Welcome to the community 🤓If you would like to continue contributing to open source and would like to do it with an awesome inclusive community, you should join our Discord chat and our GitHub Organisation - we help and encourage each other to contribute to open source little and often 🤓 . Any questions let us know.
How can I fix this? I would also be open to suggestions on my work. |
Sorry @Keshraf for my slow reply, I will take a look today |
I just saw your comment @Keshraf, it would be better to fix it in this PR |
Okay @eddiejaoude, I will fix it in this PR 👍 |
Welcome @Keshraf! Thank you so much for your first pull request! |
Sorry @Keshraf I just realised we might be talking about different menu buttons. Are you talking about the light/dark mode or the "hamburger" one? If for the existing button that is not required for this PR, sorry I assumed you meant for your new button Please clarify and we can hopefully get this merged soon, thanks 👍 |
I was talking about the "hamburger menu" button. Should I add it to this PR? |
Yes please, if you can add it to this PR as it is related to the changes here 👍 Sorry you have some conflicts to resolve now because other PRs have been merged in |
Fixes Issue
Closes #1838
Changes proposed
Check List (Check all the applicable boxes)
Screenshots
Note to reviewers
I found that the menu button for mobile-view doesn't work, should I create a new issue and PR for it?