[TreeView] Migrate TreeItem to emotion by siriwatknp · Pull Request #25835 · mui/material-ui

siriwatknp · 2021-04-19T04:01:17Z

I have followed (at least) the PR section of the contributing guide.

One chunk of #24405

mui-pr-bot · 2021-04-19T04:05:27Z

Details of bundle changes

@material-ui/lab: parsed: +0.25% , gzip: +0.16%

Generated by 🚫 dangerJS against a3234ed

…erride

mnajdova

This is a bit trickier. The only problem I see is with the styles for the iconContainer and the label, but it is solvable. Let me know if you need some more help.

siriwatknp · 2021-04-25T01:09:22Z

@oliviertassinari one question about theme styleOverrides for pseudo class. Is this the correct way to do in theme?

createMuiTheme({
  components: {
    MuiTreeItem: {
      styleOverrides: {
        root: {
          '& .Mui-focused': {
            ...styles
          },
          '& .Mui-expanded': {
            ...styles
          }
        }
      }
    }
  }
})

oliviertassinari · 2021-04-25T16:34:37Z

@siriwatknp Yes

mnajdova · 2021-04-26T12:00:01Z

@oliviertassinari one question about theme styleOverrides for pseudo class. Is this the correct way to do in theme?

createMuiTheme({
  components: {
    MuiTreeItem: {
      styleOverrides: {
        root: {
          '& .Mui-focused': {
            ...styles
          },
          '& .Mui-expanded': {
            ...styles
          }
        }
      }
    }
  }
})

@siriwatknp just watch out for the spacings, I believe it should be:

createMuiTheme({
  components: {
    MuiTreeItem: {
      styleOverrides: {
        root: {
-         '& .Mui-focused': {
+         '&.Mui-focused': {
            ...styles
          },
-         '& .Mui-expanded': {
+         '&.Mui-expanded': {
            ...styles
          }
        }
      }
    }
  }
})

mnajdova · 2021-04-26T12:02:19Z

      backgroundColor: `var(--tree-view-bg-color, ${theme.palette.action.selected})`,
      color: 'var(--tree-view-color)',
    },
+    '& $label': {


Should we use the global class here? For example:

Suggested change

'& $label': {

'& .MuiTreeItem-label': {

Or even better:

Suggested change

'& $label': {

'& .${treeItemClasses.label}': {

@mnajdova similar to @oliviertassinari suggestion, but there are many places still using $ in this file so I think it might be better to update the whole demo in another PR. is that okay?

ok, got it, sorry missed previous question I guess :)

siriwatknp · 2021-04-26T13:26:07Z

@oliviertassinari one question about theme styleOverrides for pseudo class. Is this the correct way to do in theme?

createMuiTheme({
  components: {
    MuiTreeItem: {
      styleOverrides: {
        root: {
          '& .Mui-focused': {
            ...styles
          },
          '& .Mui-expanded': {
            ...styles
          }
        }
      }
    }
  }
})

@siriwatknp just watch out for the spacings, I believe it should be:

createMuiTheme({
  components: {
    MuiTreeItem: {
      styleOverrides: {
        root: {
-         '& .Mui-focused': {
+         '&.Mui-focused': {
            ...styles
          },
-         '& .Mui-expanded': {
+         '&.Mui-expanded': {
            ...styles
          }
        }
      }
    }
  }
})

Got your point, do I need to change? because right now the pseudo styles is at Content, not Root.

const StyledTreeItemContent = experimentalStyled(...)({
  [`&.${treeItemClasses.disabled}`]: {
    ...
  },
  [`&.${treeItemClasses.focused}`]: {
    ...
  },
  [`&.${treeItemClasses.selected}`]: {
    ...
  },
})

mnajdova · 2021-04-26T14:35:31Z

Got your point, do I need to change? because right now the pseudo styles is at Content, not Root.

I would expect in that case maybe either :

createMuiTheme({
  components: {
    MuiTreeItem: {
      styleOverrides: {
        content: { // target content
          '&.Mui-focused': {
            ...styles
          },
           '&.Mui-expanded': {
            ...styles
          }
        }
      }
    }
  }
})

or this

createMuiTheme({
  components: {
    MuiTreeItem: {
      styleOverrides: {
        root: {
          '& .MuiTreeItem-content.Mui-focused': {
            ...styles
          },
           '& .MuiTreeItem-content.Mui-expanded': {
            ...styles
          }
        }
      }
    }
  }
})

siriwatknp · 2021-04-26T14:40:54Z

Got your point, do I need to change? because right now the pseudo styles is at Content, not Root.

I would expect in that case maybe either :

createMuiTheme({
  components: {
    MuiTreeItem: {
      styleOverrides: {
        content: { // target content
          '&.Mui-focused': {
            ...styles
          },
           '&.Mui-expanded': {
            ...styles
          }
        }
      }
    }
  }
})

or this

createMuiTheme({
  components: {
    MuiTreeItem: {
      styleOverrides: {
        root: {
          '& .MuiTreeItem-content.Mui-focused': {
            ...styles
          },
           '& .MuiTreeItem-content.Mui-expanded': {
            ...styles
          }
        }
      }
    }
  }
})

Yep, I am good with this.

mnajdova

👍

siriwatknp added 2 commits April 19, 2021 10:53

migrate TreeItem to emotion

0f8a6b4

run prettier and lint

da2a7b2

siriwatknp commented Apr 19, 2021

View reviewed changes

Comment thread packages/material-ui-lab/src/TreeItem/TreeItem.d.ts

siriwatknp commented Apr 19, 2021

View reviewed changes

Comment thread test/utils/describeConformanceV5.js Outdated

bring back api docs

e488cb8

siriwatknp mentioned this pull request Apr 19, 2021

Migrate all components to emotion #24405

Closed

siriwatknp added 2 commits April 19, 2021 13:46

styled label and iconContainer inside TreeItemContent for flat css ov…

12ca717

…erride

fix styling in demos

dda4834

siriwatknp added the scope: tree view Changes related to the tree view. This includes TreeView, TreeItem. label Apr 19, 2021

siriwatknp mentioned this pull request Apr 20, 2021

[test] Add test validator to improve DX #25854

Merged

1 task

siriwatknp added 2 commits April 20, 2021 23:01

remove validator

ce96f22

add sx to types

ecf66ed

github-actions Bot mentioned this pull request Apr 20, 2021

Update snapshots for pr/25835 eps1lon/mui-scripts-incubator#2172

Closed

bring back default prop

187bc83

siriwatknp requested a review from mnajdova April 21, 2021 16:44

mnajdova reviewed Apr 21, 2021

View reviewed changes

siriwatknp added 2 commits April 22, 2021 09:30

remove deepmerge & fix pseudo styling

9148b5f

use nested styles

9c94521

siriwatknp requested a review from mnajdova April 23, 2021 16:34

siriwatknp commented Apr 23, 2021

View reviewed changes

Comment thread docs/src/pages/components/tree-view/GmailTreeView.js

oliviertassinari requested changes Apr 24, 2021

View reviewed changes

fix as suggested

8fe61aa

oliviertassinari approved these changes Apr 25, 2021

View reviewed changes

sort asc

a3234ed

mnajdova reviewed Apr 26, 2021

View reviewed changes

mnajdova approved these changes Apr 27, 2021

View reviewed changes

mnajdova merged commit aac3bdf into mui:next Apr 27, 2021

eps1lon mentioned this pull request Apr 27, 2021

[test] Remove redundant render option #25999

Closed

mnajdova mentioned this pull request Apr 27, 2021

[test] Fix duplicate key in TreeItem test #26000

Merged

Uh oh!

Uh oh!

Conversation

siriwatknp commented Apr 19, 2021 • edited by oliviertassinari Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mui-pr-bot commented Apr 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mnajdova left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

siriwatknp commented Apr 25, 2021 • edited by oliviertassinari Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliviertassinari commented Apr 25, 2021

Uh oh!

mnajdova commented Apr 26, 2021

Uh oh!

mnajdova Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

mnajdova Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

siriwatknp Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

mnajdova Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

siriwatknp commented Apr 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mnajdova commented Apr 26, 2021

Uh oh!

siriwatknp commented Apr 26, 2021

Uh oh!

mnajdova left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

siriwatknp commented Apr 19, 2021 •

edited by oliviertassinari

Loading

mui-pr-bot commented Apr 19, 2021 •

edited

Loading

siriwatknp commented Apr 25, 2021 •

edited by oliviertassinari

Loading

siriwatknp commented Apr 26, 2021 •

edited

Loading