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

Fix customization with createTheme #1882

Merged
merged 2 commits into from Apr 10, 2022
Merged

Fix customization with createTheme #1882

merged 2 commits into from Apr 10, 2022

Conversation

garronej
Copy link
Contributor

@garronej garronej commented Mar 6, 2022

Hi,

UPTATE: Mainly, this PR fixes #1878 and #1842. Beside;

A new version of tss-react has been released, tss-react no longer requires it to be a peer dependency.
I have updated the package.json and the README.md to take advantage of this.

I also made a few more changes in the dependency requirement:

  • @emotion/react: It's a peer dependency of @mui/material and you use it directly. This means that @emotion/react should be a peer dependency of mui-datatalbes. @emotion/react is a module that uses React.Context thus there can't be multiple copies of the lib in a project: It can't be in dependencies.
  • @emotion/styled: It's a peer dependency of @mui/material but you never use it directly => It should be in devDependencies not dependencies.

This PR would make #1871 unnecessary.
I'm obviously biased being the author of tss-react but I think it's for the best since #1871 implements the automatic approach decribed in the v4 to v5 migration guide which is not optimal:

image

Best regards,

@garronej
Copy link
Contributor Author

garronej commented Mar 16, 2022

deleted as misleading

@garronej
Copy link
Contributor Author

Hi @wdh2100, @patorjk, @gregn, @gabrielliwerant, @InnuceEAN.
Sorry for pinging you all but it is very critical that this PR get merged.
Currently v4 is broken, it's not possible to customize theme using createTheme (cf. #1878 #1842).
Latest version of tss-react adds out of the box support for MUI Theme styleOverrides.

Best regards,

@garronej garronej changed the title Remove tss-react from peerDependencies Fix customization with createTheme Mar 20, 2022
@garronej
Copy link
Contributor Author

garronej commented Mar 23, 2022

@wdh2100 🙏🏻 Merge and release. mui-datatables theming capabilities are currently broken.

I understand that you want to test the changes before merging. If you can't make the time maybe just upgrade to the latest version of tss-react and release?
The rest can wait but upgrading TSS is critical.

Best regards,

package-lock.json Outdated Show resolved Hide resolved
@garronej
Copy link
Contributor Author

garronej commented Apr 3, 2022

Seems like this project is no longer actively maintained...
A workaround that I can offer (to yarn users) is to pin tss-react to the newer version in your package.json like so:

"resolutions": {
    "tss-react": "3.6.0"
},
"dependencies": {
    "tss-react": "^3.6.0"
}

Once this PR get merged you will be able to remove tss-react from your dependencies.

@ashfaqnisar
Copy link
Contributor

Hey @wdh2100, it would be awesome! If you could take a look at this PR.

@garronej
Copy link
Contributor Author

garronej commented Apr 5, 2022

An update on my side.
I am working with MUI to remove the need to explicitly provide an emotion cache.
When this effort get through the internal use of TSS will be completely transparent to mui-datatables's users.

@wdh2100
Copy link
Collaborator

wdh2100 commented Apr 6, 2022

sorry.. 😭 😭 😭
@ashfaqnisar @garronej

I will release new version it this weekend

@cloudmista
Copy link

cloudmista commented May 5, 2022

@garronej Is this documented somewhere? And is it fixed? Can seem to get the Styles working again.

@garronej garronej deleted the update_peer_deps branch May 5, 2022 22:05
@garronej
Copy link
Contributor Author

garronej commented May 5, 2022

Hi @FluxChris

Is this documented somewhere? And is it fixed? Can seem to get the Styles working again.

It should work just like it used to before things where broken by the upgrade to MUIv5

@cloudmista
Copy link

Can you tell me what dependencies versions you are using to make this work?

@garronej
Copy link
Contributor Author

garronej commented May 6, 2022

@FluxChris When I say it work like it used to that is per say assumed you have correctly updated MUI to v5 on your side.
The object accepted by createTheme have changed.
In @material-ui/core (v4) ti was: https://v4.mui.com/customization/components/#global-theme-override
Now, in @mui/material(v5) it's: https://mui.com/material-ui/customization/theme-components/

Here is a working example here

You can mess around with it:

git clone https://github.com/gregnb/mui-datatables
cd mui-datatables
npm install
npm run dev  
# Open to http://localhost:5050/#customize-styling

Good luck

@Atomico001
Copy link

@FluxChris When I say it work like it used to that is per say assumed you have correctly updated MUI to v5 on your side. The object accepted by createTheme have changed. In @material-ui/core (v4) ti was: https://v4.mui.com/customization/components/#global-theme-override Now, in @mui/material(v5) it's: https://mui.com/material-ui/customization/theme-components/

Here is a working example here

You can mess around with it:

git clone https://github.com/gregnb/mui-datatables
cd mui-datatables
npm install
npm run dev  
# Open to http://localhost:5050/#customize-styling

Good luck

Hello, i follow the example, but nothing happen (typescript give me errors, but i fix with an "any" workaround, but no style is apply

@garronej
Copy link
Contributor Author

Hi @Atomico001,
If you want me to help you I'll need to see your code.
About the typescript error, yes you need to cast as any, mui-datatable provides no types definition for customisation.

@Atomico001
Copy link

Hi @garronej ty for answer, i finally found the problem.

i was importing: import { createTheme, ThemeProvider } from '@material-ui/core/styles';
as a readme, but the correct import was:
import { ThemeProvider, createTheme } from '@mui/material/styles';

@garronej
Copy link
Contributor Author

@Atomico001 thanks for the feedback

@Atomico001
Copy link

I am trying to change the hover color with pointer.

const getMuiTheme = () =>
    createTheme({
      components: {
        //@ts-ignore
        MUIDataTableBodyCell: {
          styleOverrides: {
            root: {
              transition: 'background-color .2s',
              '&:hover': {
                cursor: "pointer",
                backgroundColor: "red"
              }
            }
          }
        },
      },
    });

The problem is only a cell is colored, how can i target the row?

federi95 pushed a commit to federi95/mui-datatables that referenced this pull request Dec 13, 2022
federi95 pushed a commit to federi95/mui-datatables that referenced this pull request Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

> **I tryed styling the Mui dataTable like you suggested and it still isn't working**
6 participants