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

Sidebar option in theming doesn't work #5426

Closed
mariaefi29 opened this issue Oct 20, 2020 · 13 comments · Fixed by #5429
Closed

Sidebar option in theming doesn't work #5426

mariaefi29 opened this issue Oct 20, 2020 · 13 comments · Fixed by #5429

Comments

@mariaefi29
Copy link

mariaefi29 commented Oct 20, 2020

What you were expecting:
As docs are saying in order to customise the sidebar and keep it responsive, you have to pass the settings in a theme prop in Admin:

import { createMuiTheme } from '@material-ui/core/styles';

const theme = createMuiTheme({
    sidebar: {
        width: 300, // The default value is 240
        closedWidth: 70, // The default value is 55
    },
});

const App = () => (
    <Admin theme={theme} dataProvider={simpleRestProvider('http://path.to.my.api')}>
        // ...
    </Admin>
);

What happened instead:
Unfortunately, it didn't work, because createMuiTheme doesn't support sidebar option.

I have found the same question in StackOverflow and there was a solution not to wrap it around createMuiTheme:

const theme = {
  sidebar: {
    width: 220, 
    closedWidth: 55, 
  },
} 

However, it doesn't work either. I got the following error:

Type '{ sidebar: { width: number; closedWidth: number; }; }' has no properties in common with type 'ThemeOptions'.

Steps to reproduce:

Modify App.tsx with the code from docs or the code in Related Code and you will see a compile error. I use Typescript. Apparently it doesn't work with TS.
Related code:
App.tsx:

import * as React from 'react'
import { Admin, Resource} from 'react-admin';
import dataProvider from './dataProvider';
import { cacheDataProviderProxy } from 'react-admin';
import { FeedList } from './components/feeds'
import Layout from './components/layout';
import { CollectionsBookmark } from '@material-ui/icons';

const customDataProvider = cacheDataProviderProxy(dataProvider('http://localhost:8080/api/v1'));

const theme = {
  sidebar: {
    width: 220,
    closedWidth: 55,
  },
}

const App = () => (
    <Admin
        theme={theme}
        dataProvider={customDataProvider}
        loginPage={false}
        title="Title"
        layout={Layout}>
        <Resource name="feeds" list={FeedList} icon={CollectionsBookmark}/>
    </Admin>
);

export default App;

Environment

  • React-admin version: ^3.9.4
  • React version: ^16.13.1
  • Browser: Google Chrome
  • Stack trace (in case of a JS error):
TypeScript error in /Users/Mariaefi/Documents/react/admin-is/src/App.tsx(21,9):
Type '{ sidebar: { width: number; closedWidth: number; }; }' has no properties in common with type 'ThemeOptions'.  TS2559

    19 | const App = () => (
    20 |     <Admin
  > 21 |         theme={theme}
       |         ^
    22 |         dataProvider={customDataProvider}
    23 |         loginPage={false}
@djhi
Copy link
Collaborator

djhi commented Oct 20, 2020

Thanks for reporting this. This is a documentation issue. Indeed, in order to override a component styles, you'll have to use the overrides key of the MUI theme:

const theme = {
  overrides: {
    RaSidebar: {
      width: 220,
      closedWidth: 55,
    },
  },
}

See this article which explains this in details: https://marmelab.com/blog/2020/09/11/react-admin-tutorials-build-your-own-theme.html

@mariaefi29
Copy link
Author

mariaefi29 commented Oct 20, 2020

Thank you for a good blog post! I will check it out!

However, with this new proposed style I have the following TS error:

Type '{ overrides: { RaSidebar: { width: number; closedWidth: number; }; }; }' is not assignable to type 'ThemeOptions'.
  Types of property 'overrides' are incompatible.
    Type '{ RaSidebar: { width: number; closedWidth: number; }; }' has no properties in common with type 'Overrides'.

@djhi
Copy link
Collaborator

djhi commented Oct 20, 2020

That's because we don't export a custom theme type yet. For the time being, you can try this not ideal workaround:

import { ThemeOptions as MuiThemeOptions } from "@material-ui/core";

export interface ThemeOptions extends MuiThemeOptions {
  overrides?: {
    [key: string]: any;
  };
}

const theme: ThemeOptions = {
  overrides: {
    RaSidebar: {
      size: 300
    }
  }
};

@mariaefi29
Copy link
Author

mariaefi29 commented Oct 20, 2020

Thank you! I have tried that, but unfortunately, it didn't work. The app compiled, but the sidebar had the same default size/width and for some reason all my colors have changes. I had blue-ish design, but now everything is red.

Something weird is going on, it seems like it overrides other properties and themes as well :(

image

@djhi
Copy link
Collaborator

djhi commented Oct 20, 2020

Can you set up a codesandbox showing the issue?

@mariaefi29
Copy link
Author

Is this ok? https://codesandbox.io/s/cranky-sanderson-2l2tm?fontsize=14&hidenavigation=1&theme=dark

As you can see, the size doesn't change a bit (even if you put width/size, doesn't matter), however, the colors changed.

You can remove the theme and see how it was before. But most importantly, I can't make the Sidebar responsive with a custom width easily.

@djhi
Copy link
Collaborator

djhi commented Oct 20, 2020

Yes, thanks a lot. And sorry about the bad tips...

About the theme

If you want to keep the default theme but override/extend some parts, this will make sure you don't loose our defaults such as the colors:

import { defaultTheme } from "react-admin";

export interface CustomThemeOptions extends MuiThemeOptions {
  overrides?: {
    [key: string]: any;
  };
}

const theme: CustomThemeOptions = {
  ...defaultTheme,
  overrides: {
    ...defaultTheme.overrides,
    RaSidebar: {
      ...
    }
  }
};

The readon we don't merge the custom theme with our default one ourselves is that you may provide us with a complete theme returned by createMuiTheme. Not sure it's a good reason though. @fzaninotto ?

About the Sidebar width

The documentation is even more misleading than I thought. The <SideBar> component simply accept two props for sizes:

  • size: number: the size when open
  • closedSize: number: The size when closed

@mariaefi29
Copy link
Author

mariaefi29 commented Oct 20, 2020

Thanks a lot! I'll keep that in mind!

I saw those props (size, closedSize), but they do not work :(

https://codesandbox.io/s/cranky-sanderson-2l2tm?fontsize=14&hidenavigation=1&theme=dark

I even tried to have my own SideBar and put the size parameter as a prop, nothing( still 240px width

@djhi
Copy link
Collaborator

djhi commented Oct 20, 2020

So after more digging, it seems the documentation is right but we haven't provided types for our theme... 😫 In the mean time, you can do:

import { ThemeOptions } from '@material-ui/core/styles';

export interface CustomThemeOptions extends ThemeOptions {
  sidebar?: {
    width?: number;
    closedWidth?: number;
  };
}

const theme: CustomThemeOptions = {
    sidebar: {
        width: 300, // The default value is 240
        closedWidth: 70, // The default value is 55
    },
};

const App = () => (
    <Admin theme={theme} dataProvider={simpleRestProvider('http://path.to.my.api')}>
        // ...
    </Admin>
);

@mariaefi29
Copy link
Author

mariaefi29 commented Oct 20, 2020

@djhi Thank you so much!

It works! I added ..defaultTheme though as well, because without that it changes colors again.

I'll stay tuned for provided types then! Thanks again for your help!

I am new to react, so it's difficult more me to dig such things on my own yet(

@mariaefi29
Copy link
Author

Dear @djhi,

will you export the types, please? So it will work as it says in the docs? I thought it could be done with this issue, but the issue was closed :) I would appreciate your help! Thank you!

@djhi
Copy link
Collaborator

djhi commented Oct 21, 2020

We usually close TS issues but now that we actually ship types officially, we should probably track them correctly

@djhi djhi reopened this Oct 21, 2020
@mariaefi29
Copy link
Author

Thank you very much!

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

Successfully merging a pull request may close this issue.

2 participants