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

fix(dashboard): inconsistent interaction for dashboard profile menu options #4422

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

amitamrutiya
Copy link

Fix Issue #4393

Make the entire MenuTile clickable while maintaining the existing functionality. Additionally, I have added text Light Mode so that users can see light mode text when the dashboard is in dark mode, and dark mode text when the dashboard is in light mode.

Signed-off-by: amitamrutiya2210 <amitamrutiya2210@gmail.com>
@siddhart1o1
Copy link
Member

Thanks for the PR.

I tried it locally, and when I used it for the first time and clicked on the theme toggle, it stayed in light mode, but the text changed.

image image

@siddhart1o1
Copy link
Member

siddhart1o1 commented May 5, 2024

Also, in dark mode, it shows "light mode" and in light mode, it shows "dark."
image

image

@amitamrutiya
Copy link
Author

Also, in dark mode, it shows "light mode" and in light mode, it shows "dark."

I have explicitly replaced this text with an icon.

I tried it locally, and when I used it for the first time and clicked on the theme toggle, it stayed in light mode, but the text changed.

It's weird. I am not able to reproduce this same error.

@siddhart1o1
Copy link
Member

siddhart1o1 commented May 6, 2024

It's weird. I am not able to reproduce this same error.

I am using Brave; maybe try in a private window of Brave so you don't have cookies or other data stored to mimic first-time usage.

@amitamrutiya
Copy link
Author

@siddhart1o1 I tried in brave private window still not able to reproduce this error 😟

@siddhart1o1
Copy link
Member

Thanks for the PR.

I tried it locally, and when I used it for the first time and clicked on the theme toggle, it stayed in light mode, but the text changed.

image image

Here is how you can fix this issue: the mode is not just dark and light, but it can also be system, so we need to handle that case as well.

Something like

export default function ColorSchemeToggle() {
  const { mode, setMode, systemMode } = useColorScheme()
  const currentMode = mode === "system" ? systemMode : mode   // if mode is system use `systemMode`

  return (
    <MenuItem
      onClick={() => {
        if (currentMode === "light") {
          setMode("dark")
        } else {
          setMode("light")
        }
      }}
      sx={{
        display: "flex",
        justifyContent: "space-between",
        fontWeight: "500",
      }}
    >
      {currentMode === "light" ? "Light Mode" : "Dark Mode"}.        // Also if currentMode is light show `Light Mode` text same for dark i think this is better for understanding.
      {currentMode === "light" ? <LightModeIcon /> : <DarkModeRoundedIcon />}
    </MenuItem>
  )
}

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.

None yet

2 participants