Dark toggle mode implemented , most demanding need of an engineer#1
Dark toggle mode implemented , most demanding need of an engineer#1Failureguy94 wants to merge 1 commit intoSuperexMack:masterfrom
Conversation
|
@Failureguy94 is attempting to deploy a commit to the Mack Walker Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR implements a dark mode toggle feature for the Callify frontend using the next-themes library. It adds a ThemeProvider wrapper at the root layout and a ThemeToggle button in the navbar, then applies Tailwind CSS dark:* utility classes across all pages and components.
Changes:
- Added
next-themesdependency and createdThemeProvider/ThemeTogglecomponents; wrapped the app layout withThemeProviderand addedsuppressHydrationWarningto<html> - Applied
dark:Tailwind utility classes across all frontend pages and components (Navbar, Footer, TopPage, Features, BottomContainer, HowtoUser, roomsection, Sender, Receiver) - Added unrelated backend changes:
"type": "module", adevscript, and TypeScript devDependencies to the backendpackage.json
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
Frontend/client/package.json |
Adds next-themes dependency |
Frontend/client/package-lock.json |
Lock file updated for next-themes |
Frontend/client/app/layout.tsx |
Wraps app with ThemeProvider, adds suppressHydrationWarning |
Frontend/client/app/globals.css |
Adds body transition for smooth theme switching |
Frontend/client/app/Components/ThemeProvider.tsx |
New wrapper around NextThemesProvider |
Frontend/client/app/Components/ThemeToggle.tsx |
New sun/moon toggle button using next-themes |
Frontend/client/app/Components/Navbar.tsx |
Adds dark classes and integrates ThemeToggle |
Frontend/client/app/Components/TopPage.tsx |
Adds dark classes; refactors video muted handling |
Frontend/client/app/Components/Footer.tsx |
Adds dark classes; converts to client component with year state |
Frontend/client/app/Components/Features.tsx |
Adds dark classes |
Frontend/client/app/Components/BottomContainer.tsx |
Adds dark classes; refactors CTA buttons from <button> to <Link> |
Frontend/client/app/Components/HowtoUser.tsx |
Adds dark class to heading |
Frontend/client/app/roomsection/page.tsx |
Adds dark classes to form elements |
Frontend/client/app/Sender/[id]/page.tsx |
Adds dark border classes to video containers |
Frontend/client/app/Receiver/[id]/page.tsx |
Adds dark border classes to video containers |
Backend/package.json |
Adds "type": "module", dev script, and unrelated TypeScript devDependencies |
Backend/package-lock.json |
Lock file updated for backend devDependencies |
Files not reviewed (2)
- Backend/package-lock.json: Language not supported
- Frontend/client/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| "devDependencies": { | ||
| "@types/node": "^25.3.3", | ||
| "ts-node": "^10.9.2", | ||
| "tsx": "^4.21.0", | ||
| "typescript": "^5.9.3" | ||
| } |
There was a problem hiding this comment.
The PR adds TypeScript-related devDependencies (@types/node, ts-node, tsx, typescript) to the backend, but the backend has no TypeScript files (only app.js). These packages appear unrelated to the dark mode feature described in this PR. Adding "type": "module" and the dev script can be justified as a bug fix (since app.js already uses ES module syntax), but the TypeScript toolchain deps add unnecessary bloat to the backend's devDependencies with no current use case.
|
|
||
| <div className="p-2 md:w-[70%] w-full md:h-[500px] h-[200px] md:mt-3 flex items-center justify-center"> | ||
| <video muted autoPlay={true} loop src={"/videostream.mp4"}></video> | ||
| <video ref={videoRef} autoPlay loop playsInline src={"/videostream.mp4"}></video> |
There was a problem hiding this comment.
The muted attribute has been removed from the <video> JSX element and replaced with a programmatic videoRef.current.muted = true inside useEffect. Without muted in the JSX, the server-side rendered HTML won't include the muted attribute. This means the video element will initially be rendered without muted, which browsers require for autoplay to work. Before the useEffect fires on the client, the autoplay attempt may be blocked by the browser's autoplay policy. The muted attribute should be kept directly on the <video> element as well (e.g., muted={true} or just muted), and the useEffect can remain as a workaround for React's known issue with the muted prop not being set as an HTML attribute.
| <video ref={videoRef} autoPlay loop playsInline src={"/videostream.mp4"}></video> | |
| <video ref={videoRef} autoPlay loop playsInline muted src={"/videostream.mp4"}></video> |
| const [year, setYear] = useState<number | null>(null) | ||
|
|
||
| useEffect(() => { | ||
| setYear(new Date().getFullYear()) | ||
| }, []) |
There was a problem hiding this comment.
The Footer component was converted to "use client" and the copyright year was changed from the simple {new Date().getFullYear()} to a useState/useEffect pattern that initializes year as null. This causes the copyright year to briefly appear blank (since year ?? "" renders an empty string) until after the component mounts and the effect runs. The original new Date().getFullYear() would render the same value on both server and client, so there's no hydration mismatch risk. The year ?? "" approach needlessly introduces a visual flash and layout shift. The "use client" directive was still correctly needed for the dark mode classes to work with next-themes, but the year can simply remain as {new Date().getFullYear()}.
|
|
||
| <div className="md:w-[60%] w-full"> | ||
| <p className="text-[20px] text-center">Connect to each other to share thoughts and memories or for creatng future</p> | ||
| <p className="text-[20px] text-center dark:text-gray-300">Connect to each other to share thoughts and memories or for creatng future</p> |
There was a problem hiding this comment.
There is a spelling error in this line: "creatng" should be "creating".
| <p className="text-[20px] text-center dark:text-gray-300">Connect to each other to share thoughts and memories or for creatng future</p> | |
| <p className="text-[20px] text-center dark:text-gray-300">Connect to each other to share thoughts and memories or for creating future</p> |
|
@Failureguy94 Thanks for the PR, man. I actually found your PR really cool and useful. Just give me some time I’ll review it soon. Thanks for your time and patience. |
|
@Failureguy94 Can you please share a ss or a short demo of the changes? The audio unmute feature is okay I understood that. But I’d like to see the UI changes you made, the dark theme. |


Was trying the product and did not find the dark mode ,so implemented one