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

Bug / Breaking change without major for useTheme behaviour #9687

Closed
nihauc12 opened this issue Feb 28, 2024 · 5 comments
Closed

Bug / Breaking change without major for useTheme behaviour #9687

nihauc12 opened this issue Feb 28, 2024 · 5 comments
Labels

Comments

@nihauc12
Copy link

What you were expecting:
Upgrading from 4.15.x to latest.
I have a base code that uses useTheme to change the palette of colors from one to another.

const [theme, setTheme] = useTheme();
setTheme({...myDefaultTheme, palette: { ...newPalette}})

Expected result: palette of color changes

What happened instead:
The palette of color does not change

Misc informations
In addition to the previous problem, here is a related one:
I have no darktheme configured so the code always return 'light' instead of the theme stored in the localstorage which i guess is a bug too.
Seems to come from this PR (4.16.3) https://github.com/marmelab/react-admin/pull/9503/files.

I checked the changelog between my 2 versions and there is no mention of this behaviour change (also should be major if so right ?)

Question regarding v5
I see in the code comments that the code I use is deprecated and will change in v5 for only being able to choose between 'light' and 'dark' preconfigured theme. In my use case I have a bunch of themes let's say 10 and i want the use to be able to switch between them with a select input. Not sure if this use case is covered by the new API.

Let me know if you need more precision / information.

@fzaninotto
Copy link
Member

You're right, #9503 seems to have broken the old behavior, sorry about that.

You can fix it on your side by setting the light and dark themes in the <Admin> component. As for changing the theme, yes it's possible (you can see an example in the e-commerce demo.

const [themeName] = useStore<ThemeName>('themeName', 'soft');
const lightTheme = themes.find(theme => theme.name === themeName)?.light;
const darkTheme = themes.find(theme => theme.name === themeName)?.dark;

It is indeed a breaking change and we should fix it in 4.16. However, there is a workaround, passing a theme object in useTheme won't be supported in 5.x, and fixing it in master then backporting the changes to next will break next.

So I'm -1 for fixing this bug.

@fzaninotto fzaninotto added the bug label Feb 28, 2024
@nihauc12
Copy link
Author

heyo, no worries, thx for the quick answer and workaroung

@jjsmclaughlin
Copy link

I think you need to bear in mind that people installing Symfony / api-platform from the docs at present end up with a broken system: https://stackoverflow.com/questions/77840771/error-while-generate-an-admin-interface-using-api-platform-admin-and-react-admin and a component used with just an entrypoint property, as should work, errors out the whole app.

jjsmclaughlin added a commit to jjsmclaughlin/docs that referenced this issue Mar 12, 2024
Currently it is necessary to provide valid 'theme' and 'darkTheme' props to <HydraAdmin> or it fails with no useful error message.

- https://stackoverflow.com/questions/77840771/error-while-generate-an-admin-interface-using-api-platform-admin-and-react-admin

- marmelab/react-admin#9687
@fzaninotto
Copy link
Member

This seems to be fixed in API Platform v3.4.6, can you confirm?

https://github.com/api-platform/admin/releases/tag/v3.4.6

@jjsmclaughlin
Copy link

I can confirm. Great. Thanks. Please disregard my comment and thanks for fixing it.

@djhi djhi closed this as completed May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants