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

Proll 158/dark mode changing #8

Merged
merged 31 commits into from
Apr 26, 2024
Merged

Conversation

Fermatka
Copy link
Collaborator

@Fermatka Fermatka commented Apr 16, 2024

Tasks:

  • Add functionality to toggle button in Navbar
  • Add dark mode to Navbar
  • Add dark mode to main page
  • Add dark mode to PianoRoll page
  • Add dark mode to Custom Solutions page
  • Add dark mode to Contact page
  • Refactor icons to enable color changing
  • Solve problem with RoundedTextComponent

Please remember that there was design update, which I'm going to do in next task:)
task link

Done:)

@Fermatka Fermatka self-assigned this Apr 16, 2024
Copy link

@TheMankel TheMankel left a comment

Choose a reason for hiding this comment

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

Well done! 🔥 Two notes:

  1. The pandas and AWS icons are poorly visible in dark mode and look slightly different in Figma. In addition, with the Redis icon the white color is visible in places that should be transparent
    image

  2. Second thing, there are still some warnings in the console:
    image

Comment on lines -23 to 26
<div className="bg-neutralLight-neutral90 rounded-[2rem] md:min-w-[36.225rem] overflow-hidden">
<div className="bg-neutralLight-neutral90 dark:bg-neutralDark-neutral90 rounded-[2rem] md:min-w-[36.225rem] overflow-hidden">
<Image
height={283}
width={343}

Choose a reason for hiding this comment

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

It is possible that downloading a photo with a background was not the best idea, because in dark mode we have a light background and if Figma it changes with theme. You can try swapping image conditionally or change the photo and add the background manually

image

@@ -5,7 +5,7 @@ import solutionsTexts from "@/data/solutionsSectionTexts";
const SolutionsSection = ({ version }) => {
const { title } = solutionsTexts[version];
return (
<section className="bg-neutralLight-neutral10 w-full flex justify-center">
<section className="bg-neutralLight-neutral10 dark:bg-neutralDark-neutral90 w-full flex justify-center">
<div className="pt-[5.88rem] pb-[5.88rem] md:pt-[7.75rem] md:pb-[7.75rem] flex flex-col justify-center items-center gap-11 max-w-[49.375rem]">
<h4 className="text-neutralLight-neutral100 h2SB xl:h2 justify-center items-center text-center">
{title}

Choose a reason for hiding this comment

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

The second link icon should also be visible for both themes, and the background colors for the dark mode don't match the design in Figma. Here's figma:

image

@@ -3,7 +3,7 @@ import React from "react";
const RoundedRectangle = () => {
return (
<div className="relative w-full h-[2rem] md:h-[6.25rem] bg-neutralLight-neutral10">

Choose a reason for hiding this comment

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

The dark:bg-neutralLight-neutral20 is missing here, so in dark mode it looks like not rounded

image

Comment on lines 6 to 21
const [darkMode, setDarkMode] = useState(true);

useEffect(() => {
const theme = localStorage.getItem("theme");
if (theme === "dark") setDarkMode(true);
}, []);

useEffect(() => {
if (darkMode) {
document.documentElement.classList.add("dark");
localStorage.setItem("theme", "dark");
} else {
document.documentElement.classList.remove("dark");
localStorage.setItem("theme", "light");
}
}, [darkMode]);

Choose a reason for hiding this comment

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

This code uses LocalStorage, but when the page is refreshed, dark mode is set as the default. So it looks like there is a problem somewhere in the logic 🤔

@roszcz
Copy link
Member

roszcz commented Apr 23, 2024

Looks great! 🕶️

Pandas and AWS icons are hard to read - can we make them look like in figma:

image

@Fermatka
Copy link
Collaborator Author

Looks great! 🕶️

Pandas and AWS icons are hard to read - can we make them look like in figma:
I'll try my best:)

@Fermatka
Copy link
Collaborator Author

Fermatka commented Apr 24, 2024

@roszcz , please take a look when you have a moment, at the CustomSolutions subpage and the icons in Our technologies. They appear a little differently than in Figma in DarkMode, but they are visible. Should it stay like this, or should I change it to preserve the color?
Algrtm _ Algorithmic solutions - Google Chrome 24 04 2024 13_35_24
Algrtm _ Algorithmic solutions - Google Chrome 24 04 2024 13_35_32

@Fermatka Fermatka requested a review from TheMankel April 24, 2024 11:41
Copy link

@TheMankel TheMankel left a comment

Choose a reason for hiding this comment

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

I noticed two more things:

  1. These shapes should be white only in light mode and #1C1C1C in dark mode
    image
  2. The "Let's talk" button should be white in dark mode
    image

@roszcz
Copy link
Member

roszcz commented Apr 24, 2024

Icons look good for me, I think we can keep redis as is 👍

@Fermatka Fermatka requested a review from TheMankel April 24, 2024 21:13
@Fermatka Fermatka merged commit d846093 into develop Apr 26, 2024
1 check passed
@Fermatka Fermatka deleted the PROLL-158/dark_mode_changing branch April 26, 2024 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants