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

[core] Optimize Theme Mode Selector in Header #4151

Closed
douglaszaltron opened this issue Sep 24, 2024 · 8 comments · Fixed by #4340
Closed

[core] Optimize Theme Mode Selector in Header #4151

douglaszaltron opened this issue Sep 24, 2024 · 8 comments · Fixed by #4340
Assignees
Labels
component: layout This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature scope: toolpad-core Abbreviated to "core"

Comments

@douglaszaltron
Copy link

douglaszaltron commented Sep 24, 2024

Summary

When we have the ability in the theme to select mode (system, dark, and light), a toggle button is created in the header. However, this element occupies precious space in the header that could be used for other types of configurations. The proposal is to follow the MUI documentation style, where there is a gear button that contains configuration options, including the mode selector, along with other settings.

For example: language preferences, display preferences, among others.

Examples

Captura de Tela 2024-09-24 às 10 48 13 Captura de Tela 2024-09-24 às 10 48 17

Motivation

No response

Search keywords: theme

@douglaszaltron douglaszaltron added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 24, 2024
@Janpot
Copy link
Member

Janpot commented Sep 24, 2024

Assuming there are going to be many different ways of how users would want this to be executed (*), perhaps step 1 should be to create the minimum API necessary to customize this in user land.
It's already possible to build your own theme toggle with the useColorScheme hook. The one thing missing then is a way to hide the theme toggle in the header.

cc @apedroferreira Perhaps a quick win here could be to add a slot for the theme toggle?


(*) I'm imagining

  • a toggle button
  • a drawer as per this issue
  • a button with popover
  • on a separate settings page
  • no toggle at all, just follow the system preference
  • ...

@Janpot Janpot added enhancement This is not a bug, nor a new feature component: DashboardLayout and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 24, 2024
@apedroferreira
Copy link
Member

Assuming there are going to be many different ways of how users would want this to be executed (*), perhaps step 1 should be to create the minimum API necessary to customize this in user land. It's already possible to build your own theme toggle with the useColorScheme hook. The one thing missing then is a way to hide the theme toggle in the header.

cc @apedroferreira Perhaps a quick win here could be to add a slot for the theme toggle?

(*) I'm imagining

  • a toggle button
  • a drawer as per this issue
  • a button with popover
  • on a separate settings page
  • no toggle at all, just follow the system preference
  • ...

Sounds good, we have 2 slots in the header right now (account + additional actions), but the theme is outside of them.
I guess we should have the minimum amount of slots in the header than can be completely overriden.

@ebengtso
Copy link

Hi, I would like to restore the user theme's choice when user logins back. Is it possible to provide a Slotprops for Theme's user action?
Thanks

@Janpot
Copy link
Member

Janpot commented Sep 30, 2024

@ebengtso Can you elaborate a bit more on what you're trying to do?

Are you storing the dark/light mode choice in the user record on the backend? You should be able to already achieve that functionality with mode and setMode returned by the useColorScheme hook.

@ebengtso
Copy link

ebengtso commented Sep 30, 2024

@ebengtso Can you elaborate a bit more on what you're trying to do?

Are you storing the dark/light mode choice in the user record on the backend? You should be able to already achieve that functionality with mode and setMode returned by the useColorScheme hook.

Thanks. Two distinct issues:

  1. I had a wrapping ThemeProvider. I removed it and it worked (as a cookie). It was disturbing the AppProvider.

  2. In the below I was expecting debug to print, but it's not printing when I switch the theme. For this reason I cannot save the mode on user change.

const {mode, setMode} = useColorScheme();

useEffect(() => {
console.debug(mode)
}, [mode]);

@Janpot
Copy link
Member

Janpot commented Oct 1, 2024

@ebengtso Here's a stackblitz that demonstrates useColorScheme. Make sure to call it in a component that's rendered under the DashboardLayout. I've opened a ticket to improve DX when it's not the case.

@douglaszaltron
Copy link
Author

In this case, where the hook is exported, the dashboard could have the option of a slot for the theme. This way, we could set it to null and customize it through the action or account slot.

@oliviertassinari oliviertassinari added component: layout This is the name of the generic UI component, not the React module! and removed component: DashboardLayout labels Nov 4, 2024
@github-project-automation github-project-automation bot moved this from In progress to Completed in MUI Toolpad public roadmap Nov 7, 2024
Copy link

github-actions bot commented Nov 7, 2024

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@douglaszaltron How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: layout This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature scope: toolpad-core Abbreviated to "core"
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

6 participants