feat: upgrade material-ui to 4.9.2 by vedrani · Pull Request #1101 · toptal/picasso

vedrani · 2020-02-10T21:15:54Z

FX-740

Description

Upgrade to 4.9.2

TODO

fix tests
fix visuals
fix problem with types - “Type instantiation is excessively deep and possibly infinite” but only in a large codebase microsoft/TypeScript#34933

denieler

I wish it will be a single change we need to do 🤞

havenchyk · 2020-02-12T06:41:37Z

docs/contribution/css-naming.md

 import React from 'react'
 import cx from 'classnames'
-import { capitalize } from '@material-ui/core/utils/helpers'
+import capitalize from '@material-ui/core/utils/capitalize'


did they remove export of this helper from utils?

Thanks, seems they export is as named export in utils
https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/utils/index.js#L1

But seems they forgot to update TS file also :(
https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/utils/index.d.ts

havenchyk · 2020-02-12T06:43:00Z

packages/picasso/src/Checkbox/__snapshots__/test.tsx.snap

          />
          <div
            class="Checkbox-uncheckedIcon"
+            font-size="default"


hmm, font-size? I've never seen something like this

yeah, interesting how they use it

It's part of MUI upgrade, do you think I should re-check impact on Picasso?

no, don't think so, just interesting 😃

havenchyk · 2020-02-12T06:43:32Z

packages/picasso/src/Modal/styles.ts

    },
    container: {},
    paper: {
+      maxHeight: 'calc(100% - 6rem)',


this change is the scariest for me 😃

Why? It was like that before and they decreased it to 'calc(100% - 64px)', so I defined it as 96px as it was.
mui/material-ui#17867

ah, ok 😄 it was just missing for me from the style and "bam!" it's here 😁

havenchyk · 2020-02-12T06:46:58Z

packages/picasso/src/Slider/Slider.tsx

-    | string
-    | ((value: number, index: number) => React.ReactNode)
-  index?: number
+  valueLabelDisplay: ValueLabelDisplay


am I right that all those changes only for internal parts of picasso Slider?

Yes, MUi changed what is sent to Slider Value Label component

borisyordanov · 2020-02-12T08:14:38Z

packages/picasso/src/OutlinedInput/OutlinedInput.tsx

    Omit<
      InputHTMLAttributes<HTMLInputElement>,
-      'value' | 'defaultValue' | 'size'
+      'value' | 'defaultValue' | 'size' | 'color'


What's the reason for this?

MUI introduced primary and secondary color that are not compatible with HTML Input Element props and as we don't support color for our components in this case I omit color from our Props.

denieler · 2020-02-12T09:28:11Z

packages/picasso/src/Checkbox/__snapshots__/test.tsx.snap

          />
          <div
            class="Checkbox-uncheckedIcon"
+            font-size="default"


yeah, interesting how they use it

denieler · 2020-02-12T09:28:55Z

packages/picasso/src/Modal/__snapshots__/test.tsx.snap

    <div
      class="MuiDialog-container PicassoModal-container MuiDialog-scrollPaper"
-      role="document"
+      role="none presentation"


I like to don't care about such things 👍 thank you MUI 😄

denieler · 2020-02-12T09:30:21Z

packages/picasso/src/Slider/story/Tooltip.example.jsx

 const TooltipExample = () => {
  return (
-    <Container>
+    <Container padded='small'>


denieler · 2020-02-12T09:30:54Z

packages/picasso/src/Slider/Slider.tsx

  - **off** will never display
  */
-  tooltip?: 'on' | 'auto' | 'off'
+  tooltip?: ValueLabelDisplay


wouldn't it be a breaking change?

No, I just moved it above :D

picasso/packages/picasso/src/Slider/Slider.tsx

Line 12 in 51f1c79

type ValueLabelDisplay = 'on' | 'auto' | 'off'

denieler · 2020-02-12T09:31:36Z

packages/picasso/src/Tab/styles.ts

      }
    },
-    selected: {},
+    selected: {


nice refactoring 👍 thanks! ❤️

denieler · 2020-02-12T09:32:01Z

packages/picasso/src/Modal/styles.ts

    },
    container: {},
    paper: {
+      maxHeight: 'calc(100% - 6rem)',


this change is the scariest for me 😃

denieler · 2020-02-12T10:40:57Z

btw, don't forget to update package.json versions (and resolution section as well) if you didn't yet 😄

vedrani · 2020-02-12T10:43:39Z

btw, don't forget to update package.json versions (and resolution section as well) if you didn't yet 😄

Thanks will do.

vedrani · 2020-02-12T14:33:48Z

Found 1 problem. It would be great to have side-effects for all components with behaviour.

toptal-devbot · 2020-02-12T22:12:41Z

🎉 Last commit is successfully deployed 🎉

Demo is available on:

https://picasso.toptal.net/fx-740-mui-4.9-upgrade/

Your davinci-bot 🚀

vedrani requested a review from a team February 10, 2020 21:15

vedrani force-pushed the fx-740-mui-4.9-upgrade branch from 2ddc5b2 to d7955a4 Compare February 10, 2020 21:27

denieler reviewed Feb 11, 2020

View reviewed changes

toptal deleted a comment from toptal-devbot Feb 12, 2020

havenchyk reviewed Feb 12, 2020

View reviewed changes

borisyordanov self-requested a review February 12, 2020 07:42

borisyordanov approved these changes Feb 12, 2020

View reviewed changes

borisyordanov reviewed Feb 12, 2020

View reviewed changes

OleksandrNechai approved these changes Feb 12, 2020

View reviewed changes

denieler approved these changes Feb 12, 2020

View reviewed changes

vedrani force-pushed the fx-740-mui-4.9-upgrade branch from e09a75d to ae60718 Compare February 12, 2020 12:31

vedrani added 4 commits February 12, 2020 13:33

feat: upgrade material-ui to 4.9.2

ded55a1

feat: upgrade material-ui to 4.9.2

67d511f

test: update snapshots

85187a8

fix: fix Chip and Label

814724f

vedrani force-pushed the fx-740-mui-4.9-upgrade branch from ae60718 to 9760951 Compare February 12, 2020 12:33

vedrani added 5 commits February 12, 2020 15:01

fix: fix Modal and Slider

30ccfd8

fix: fix Tab

ff8305c

fix: fix Slider tooltip example

662e4ca

test: update visuals

ae092a7

fix: fix yarn lock

40f75e8

vedrani force-pushed the fx-740-mui-4.9-upgrade branch from 9760951 to 40f75e8 Compare February 12, 2020 14:01

vedrani added 2 commits February 12, 2020 22:31

fix: fix Table selected row style

dda2e4f

fix: fix TS error in example

708a1ef

vedrani merged commit 3082e50 into master Feb 13, 2020

vedrani deleted the fx-740-mui-4.9-upgrade branch February 13, 2020 08:28

Conversation

vedrani commented Feb 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

denieler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

denieler commented Feb 12, 2020

Uh oh!

vedrani commented Feb 12, 2020

Uh oh!

vedrani commented Feb 12, 2020

Uh oh!

toptal-devbot commented Feb 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vedrani commented Feb 10, 2020 •

edited

Loading