Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toggle Button for Light/Dark added #108

Merged
merged 3 commits into from
May 25, 2024

Conversation

Prashant-2024
Copy link
Contributor

@Prashant-2024 Prashant-2024 commented May 22, 2024

Issue No. #54
Toggle Button added for Light and Dark mode of application.

Copy link

vercel bot commented May 22, 2024

@Prashant-2024 is attempting to deploy a commit to the Abhijeet's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, @Prashant-2024! 🎉 Thank you for submitting your pull request. Your contribution is valuable and we appreciate your efforts to improve our project.

We will promptly review your changes and offer feedback. Keep up the excellent work! Kindly remember to check our contributing guidelines

@abhijeetnishal
Copy link
Owner

Hey @Prashant-2024, there are couple of issue in this PR:

  1. Component should be in TypeScript not JS and file should be in .tsx format for component.
  2. In the current project, we already using "next-themes" so we can implement Light/dark mode feature directly using this.
  3. The switch between light and dark mode is just button not a toggle one, use toggle button.
  4. The toggle button is present in Navbar not in page so that users can switch in any component
  5. The button should be visible, currently it's not visible size is small as it's not a toggle button.

Try to understand the project first before contributing so that you can easily understand.

@abhijeetnishal
Copy link
Owner

abhijeetnishal commented May 22, 2024

You can implement using next-themes like this:

"use client";
import { useTheme } from "next-themes";

export default function ThemeToggle() {
  const { systemTheme, theme, setTheme } = useTheme();
  const currentTheme = theme === "system" ? systemTheme : theme;

  const toggleTheme = () => {
    const newTheme = currentTheme === "dark" ? "light" : "dark";
    setTheme(newTheme);
  };

  return (
    <button
      onClick={toggleTheme}
      className="p-2 bg-gray-200 dark:bg-gray-800 rounded"
    >
      {theme === "light" ? "🌞" : "🌜"}
    </button>
  );
}

See this is easy to implement, that's why i am saying first understand the project.
Currently we can implement using this later it can be done using state management library.

@Prashant-2024
Copy link
Contributor Author

Prashant-2024 commented May 22, 2024

I'm sorry for causing this inconvenience, I missed the part where I had to implement it with typescript, it's still new to me and am currently learning it.
I did understood the feature and that's why asked to assign it to me.
The delay was caused because I had exams in between.
I will the correct the implementation ASAP, and share it with you.

@abhijeetnishal
Copy link
Owner

No problem @Prashant-2024, open source and gssoc is for learning.

@abhijeetnishal
Copy link
Owner

Hey @Prashant-2024, are you done with the changes?

@Prashant-2024
Copy link
Contributor Author

Yes, I am close to it.
I will share them by today evening.
I know i have taken long time and causing delay, for this changes. I am currently travelling and don't have access to laptop.
I will share them ASAP

@Prashant-2024
Copy link
Contributor Author

Below are the changes:

Dark Mode:

image

Light Mode:

image

@abhijeetnishal Request to look into it and let me know

// // components/ThemeToggle.tsx
// "use client";
// import { useTheme } from "next-themes";
// import { useEffect, useState } from "react";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to remove commented code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

Copy link
Owner

@abhijeetnishal abhijeetnishal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have approved this PR but i have added comments don't repeat this for other projects.

@abhijeetnishal abhijeetnishal changed the base branch from master to development May 25, 2024 16:00
@abhijeetnishal abhijeetnishal merged commit 1e43e7c into abhijeetnishal:development May 25, 2024
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants