search

helfi92 / material-ui-treeview

A React tree view for material-ui.
MIT License
47 stars 21 forks source link

Migrate to material-ui v4 #46

Closed helfi92 closed 4 years ago

helfi92 commented 5 years ago

Closes https://github.com/helfi92/material-ui-treeview/issues/36.

helfi92 commented 5 years ago

Do you know why this style broke in latest material-ui? It’d be good to know why.

edil-it-them commented 5 years ago

I added marginRight: 0 to expandIcon that overrides .MuiIconButton-edgeEnd css rule with marginRight: -12px. My commit made so many changes in yarn.lock. Is it ok?

helfi92 commented 5 years ago

I added marginRight: 0 to expandIcon that overrides .MuiIconButton-edgeEnd css rule with marginRight: -12px.

What I'm trying to understand is why it was working in previous material-ui but broke in material v4.

My commit made so many changes in yarn.lock. Is it ok?

Yeah, that's okay.

edil-it-them commented 5 years ago

Do you know why this style broke in latest material-ui? It’d be good to know why.

I'm trying to find it in material-ui changelog and documentation. Don't understand why expandIcon gets edgeEnd now. It's too late in my timezone, I'll try to find it tomorrow

helfi92 commented 5 years ago

Sounds good.

edil-it-them commented 5 years ago

This PR added edge props to IconButton in material-ui v4, default value is false. But expandIcon in ExpansionPanelSummary adds edge: 'end' to Icon as default. May be it's material-ui bug ) We can add IconButtonProps={{edge:"false"}} prop to ExpandPanelSummary and we can remove extra classes

helfi92 commented 5 years ago

We can add IconButtonProps={{edge:"false"}} prop to ExpandPanelSummary and we can remove extra classes

I like this solution. Let's do it :)

edil-it-them commented 5 years ago

I'm not sure about padding between items, but it must work after overrides in theme.js. I can't run yarn start:styleguide, cause this warning theme.js not working in http://localhost:5000/ development server(

warning

helfi92 commented 5 years ago

Does yarn run styleguidist server work?

helfi92 commented 5 years ago

Another thing you can try is rm -rf node_modules/ && yarn && yarn start:styleguide.

edil-it-them commented 5 years ago

Another thing you can try is rm -rf node_modules/ && yarn && yarn start:styleguide Does yarn run styleguidist server work?

I've tried both, not working. Catch the same error

helfi92 commented 5 years ago

I've tried both, not working. Catch the same error

Ah, it's possible that you introduced code that doesn't compile. Check git status for any code introduced by accident like an extra comma or something.

edil-it-them commented 5 years ago

import StyleGuide from 'react-styleguidist/lib/client/rsg-components/StyleGuide/StyleGuideRenderer'; Works for me

There are two more regressions Before

before

After

after

It seems like after migration css rules work different

css after

In old version .makeStyles-panelExpanded override .MuiExpansionPanel-root.Mui-expanded Should we change it?

helfi92 commented 5 years ago

In old version .makeStyles-panelExpanded override .MuiExpansionPanel-root.Mui-expanded Should we change it?

Yes please :)

edil-it-them commented 5 years ago

I followed this example to make it work correctly

edil-it-them commented 5 years ago

Is it OK now?

helfi92 commented 5 years ago

Yeah, looks good :) Do you know why this happened by any chance?

edil-it-them commented 5 years ago

React styleguide now uses Prism for codeviewer and these values are default. So I changed some of these values in theme.js In demo styleguide uses CodeMirror for codeviewer

helfi92 commented 4 years ago

Awesome work @Rolikasi. Thank you for the help!

helfi92 commented 4 years ago

Released in material-ui-treeview@4.0.0 🎉

edil-it-them commented 4 years ago

Woohoo!!!