react-admin icon indicating copy to clipboard operation
react-admin copied to clipboard

Bug / Breaking change without major for useTheme behaviour

Open nihauc12 opened this issue 5 months ago • 5 comments

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.

nihauc12 avatar Feb 28 '24 12:02 nihauc12

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.

https://github.com/marmelab/react-admin/blob/601a0d61d6c687fc8e43d03553b0501b4876d567/examples/demo/src/App.tsx#L45-L47

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 avatar Feb 28 '24 12:02 fzaninotto

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

nihauc12 avatar Feb 28 '24 15:02 nihauc12

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 <HydraAdmin> component used with just an entrypoint property, as should work, errors out the whole app.

jjsmclaughlin avatar Mar 12 '24 09:03 jjsmclaughlin

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

fzaninotto avatar Mar 12 '24 10:03 fzaninotto

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

jjsmclaughlin avatar Mar 12 '24 10:03 jjsmclaughlin