Custom theme: styleOverrides on Ra* components are not applied

[sweco-sedalh]

What you were expecting:

It being possible to override styles of Ra* components in the theme as in v3.

What happened instead:

Styles specified in the theme are not applied. Styles on Mui* components are, however, applied as expected.

Steps to reproduce:

1. Create a custom theme and pass it to
2. Add styleOverrides on react-admin components (see below)
3. Open in browser

Related code:

https://codesandbox.io/s/determined-wilson-k6ifbu?file=/src/index.tsx:722-1068

const theme = _.merge({}, defaultTheme, {
  components: {
    MuiIconButton: {
      styleOverrides: {
        root: {
          // works as expected
          color: "red"
        }
      }
    },
    RaList: {
      styleOverrides: {
        content: {
          // does not work
          backgroundColor: "red"
        }
      }
    }
  }
});

Other information:

Environment

• React-admin version: 4.0.1
• Last version that did not exhibit the issue (if applicable): 3.19.7
• React version: 17.0.2
• Browser: Chrome 100.0.4896.127
• Stack trace (in case of a JS error):

[slax57]

Hi!

I believe you're not trying to do exactly the same thing with your MUI component and your RA component, since on one hand you override the root component and on the other hand you try to override the "content" subcomponent.

The syntax you wrote used to work with MUI v4 but I believe this has changed with MUI v5.

Below is the way you can achieve this now:

const theme = _.merge({}, defaultTheme, {
  components: {
    MuiIconButton: {
      styleOverrides: {
        root: {
          // works as expected
          color: "red"
        }
      }
    },
    RaList: {
      styleOverrides: {
        root: {
          backgroundColor: "red",
          "& .RaList-content": {
            backgroundColor: "blue"
          }
        }
      }
    }
  }
});

Hope this helps.

[sweco-sedalh]

It's, or at least should be, the same thing. Though I realize my original example wasn't clear on that, I've updated the sandbox and here is a new snippet:

const theme = _.merge({}, defaultTheme, {
  components: {
    MuiButton: {
      styleOverrides: {
        root: {
          // works as expected
          border: "5px solid red"
        },
        startIcon: {
          // works as expected
          color: "red"
        }
      }
    },
    RaList: {
      styleOverrides: {
        root: {
          // works as expected
          backgroundColor: "green"
        },
        content: {
          // does not work
          backgroundColor: "red"
        }
      }
    }
  }
});

See also some of the examples from the MUI documentation: https://mui.com/material-ui/customization/theme-components/#global-style-overrides

Your code works as a workaround, however this issue should definitely not be closed as it stands IMO.

[sweco-sedalh]

I haven't fully understood how MUI wants it now in v5, but from what I understand the overridesResolver (for example https://github.com/marmelab/react-admin/blob/master/packages/ra-ui-materialui/src/list/ListView.tsx#L171) would need to be adjusted, see for a somewhat related example: https://mui.com/system/styled/#custom-components.

I believe something along these lines should work:

  overridesResolver: (props, styles) => {
    return [
      styles.root,
      {
        [`& .${ListClasses.main}`]: styles.main,
        [`& .${ListClasses.content}`]: styles.content,
        [`& .${ListClasses.actions}`]: styles.actions,
        [`& .${ListClasses.noResults}`]: styles.noResults
      }
    ];
  },

[djhi]

Thanks for the details even though I don't like it 😂 Feel free to help us fix them ;-) I'm marking this as an enhancement as there are workarounds

[sweco-sedalh]

PR created.

[fzaninotto]

All in all, I don' think this is a bug. You can override styles in react-admin components by the complete class name:

const theme = _.merge({}, defaultTheme, {
  components: {
    RaList: {
      styleOverrides: {
        root: {
          '& .RaList-content': {
            backgroundColor: "red"
          }
        }
      }
    }
  }
});

This is the way that is documented (albeit poorly) in the Theming section and in the individual component docs (ex: List view styles)

I understand that it's not MUI's way, but there are 2 reasons I don't want to go their path:

1. MUI's way means there are 2 ways to override styles. Because our approach works for MUI components, too. And having two ways of doing the same thing brings confusion.
2. Our way requires less documentation, as developers can infer the class names by inspecting the DOM.

So I think we'll stay with out approach. the Theming documentation needs to be updated, though.

[sweco-sedalh]

Hm, I'd like to counter those arguments (and add some in support of this):

1. Regardless of how many ways MUI provides I'd (and I believe I'm not alone in that) would expect react-admin, being built on MUI, to support the same ways. For good DX, something react-admin in general is very good at, it's IMO always better to be as unsurprising as possible.
2. On the contrarian, doing it the MUI way would just require a "look at the MUI docs", deviating from that route requires documentation on how react-admin is different (see previous point). Your way also requires the user to know if a substyle(?) is on the root element or a children (see the PR for an example of me already missing that), thus you'd have to document this for every substyle on every component. Sure, you can look at the DOM but that often gives an incomplete picture (are there similar substyles for other states I'm missing right now? under what specific circumstances does this substyle get added? are there other cases where this substyle is used that I don't indent to override right now?)
3. Pushing users towards this approach will in the long run bind you harder to the current structure of the HTML. Doing it the "MUI-way" gives you freedom to change between <div class="RaList-root"><div class="RaList-content"></div></div> and <div class="RaList-root RaList-content"></div> without users having to adjust their code.
4. Type support, while Fix style overrides for Ra* components #7616 doesn't add that it would be possible to add typing similar to how MUI does it

[fzaninotto]

Thanks for your input. I understand your point of view, and in my opinion there is no clear winner. We need to find the best compromise that leads to the best DX.

I updated the documentation in #7636 to make the current implementation explicit. To me, the doc is complex enough without explaining yet another way of overriding styles. And my experience with MUI style overrides is that they are a pain, as you cannot override anything without looking at their doc.

Regarding the ability to change HTML without changing the style overrides, to me that's a breaking change and it should not happen.

As for the TypeScript check, I agree that it's a big plus, but it comes with a huge cost for us. Maintaining these types in addition to the style overrides itself is really not something I want to spend my time on.

[mediafreakch]

To me it looks like the example in the "Global Theme Overrides" section in documentation is wrong:

import { defaultTheme } from 'react-admin';

const theme = {
    ...defaultTheme,
    components: {
        ...defaultTheme.components,
        RaDatagrid: {
+         styleOverrides: {
            root: {
                backgroundColor: "Lavender",
                "& .RaDatagrid-headerCell": {
                    backgroundColor: "MistyRose",
                },
            }
+        }
        }
    }
};

const App = () => (
    <Admin theme={theme}>
        // ...
    </Admin>
);

Also I noticed that the overrides in the theme only apply after refreshing the page. Hot-Reload doesn't work for me.

[mediafreakch]

Also, it looks like the function style notation with access to the theme does not work (error in console):

import { defaultTheme } from 'react-admin';

const theme = {
    ...defaultTheme,
    components: {
        ...defaultTheme.components,
        RaTabbedShowLayout: {
         styleOverrides: {
            root: {
              '& .RaTabbedShowLayout-content': ({ theme }: any) => ({
                display: 'block',
                marginTop: theme.spacing(-1),
                marginLeft: theme.spacing(-2),
                marginRight: theme.spacing(-2),
          }),
        },
      },
    },
    }
};

const App = () => (
    <Admin theme={theme}>
        // ...
    </Admin>
);

Throws:

Using kebab-case for css properties in objects is not supported. Did you mean & .RaTabbedShowLayoutContent?
at http://localhost:3000/static/js/bundle.js:39862:66
at OptionalRecordContextProvider (http://localhost:3000/static/js/bundle.js:129343:12)
at TabbedShowLayout (http://localhost:3000/static/js/bundle.js:180351:15)
...

UPDATE:

Here's how it is supposed to be and works now:

      RaTabbedShowLayout: {
         styleOverrides: {
-          root: {
-            '& .RaTabbedShowLayout-content': ({ theme }: any) => ({
+          root: ({ theme }: any) => ({
+            '& .RaTabbedShowLayout-content': {
               display: 'block',
               // account for datagrid inside Tab
               marginTop: theme.spacing(-1),
               marginLeft: theme.spacing(-2),
               marginRight: theme.spacing(-2),
-            }),
-          },
+            },
+          }),
         },
       },

[fzaninotto]

To me it looks like the example in the "Global Theme Overrides" section in documentation is wrong:

Well spotted! Would you like to open a PR to fix it?

[fzaninotto]

Fixed by #7727