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

Add more themes #9316

Merged
merged 73 commits into from
Oct 11, 2023
Merged

Add more themes #9316

merged 73 commits into from
Oct 11, 2023

Conversation

adguernier
Copy link
Contributor

@adguernier adguernier commented Sep 26, 2023

Problem

React-admin bundles only one light and one dark theme. Most users need to adapt the app theme to their brand, and the default themes aren't always a good starting point.

Solution

Build 3 more themes showing variations in fonts, spacing, border-radius, colors, etc.

Todo

  • build Nano theme
  • build Radiant theme
  • build House theme
  • provide these themes in ra-ui-materialui
  • move all theme objects, hooks and components into a theme folder
  • build a theme switcher in the e-commerce demo
  • add documentation to introduce these new themes
demo-themes.mp4

@adguernier adguernier changed the title Add more themes in e-commerce demo Add more themes Sep 27, 2023
@fzaninotto fzaninotto changed the base branch from master to next October 11, 2023 07:11
@fzaninotto fzaninotto changed the base branch from next to master October 11, 2023 07:11
@fzaninotto fzaninotto changed the base branch from master to next October 11, 2023 07:13
@fzaninotto
Copy link
Member

Loading the 3 new themes adds about 15kB to the demo, which is acceptable and doesn't justify lazy-loading.

image

@@ -38,7 +38,7 @@ const englishMessages: TranslationMessages = {
move_up: 'Move up',
move_down: 'Move down',
open: 'Open',
toggle_theme: 'Toggle Theme',
toggle_theme: 'Toggle light/dark mode',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you check the french translations?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the French translation is already "Thème clair/sombre", which is explicit.

I changed the English translation because it kind of conflicted with the "change theme" tooltip of the ThemeSwapper button in the demo. Now it is more specific.

@@ -16,7 +16,12 @@ export const LoadingIndicator = (props: LoadingIndicatorProps) => {
const theme = useTheme();
return (
<Root className={className} sx={sx}>
{loading ? (
<RefreshIconButton
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Member

Choose a reason for hiding this comment

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

LoadingIndicator needs to replace the circular arrow icon by a circular loader, and it needs to do it precisely for the loading effect to be nice. The current way of doing it didn't work with a different spacing preference in the theme, and couldn't be adjusted in the theme using component styleOverrides.

The new way of showing the circular loader is more robust, can be overridden, and adapts well to smaller font sizes (see the Nano theme).

defaultProps: {
variant: 'outlined' as const,
},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add margin between glued input

Suggested change
},
},
MuiFormControl: {
defaultProps: {
margin: 'normal' as const,
},
},

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in 0bc94a4, thanks

@@ -48,6 +48,7 @@ export const LocalesMenuButton = (props: LocalesMenuButtonProps) => {
<Root component="span">
<Button
color="inherit"
variant="text"
Copy link
Member

Choose a reason for hiding this comment

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

This is to avoid an outline when the theme sets the button variant to outlined

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants