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

[Grid] Use a unitless spacing API #14099

Merged
merged 4 commits into from
Feb 8, 2019

Conversation

ifndefdeadmau5
Copy link
Contributor

@ifndefdeadmau5 ifndefdeadmau5 commented Jan 6, 2019

See the discussion about this PR here.

Closes #12925
Closes #13967
Closes #14280

Breaking change

In order to support arbitrary spacing values and to remove the need to mentally county by 8, we are changing the spacing API:

  /**
   * Defines the space between the type `item` component.
   * It can only be used on a type `container` component.
   */
-  spacing: PropTypes.oneOf([0, 8, 16, 24, 32, 40]),
+  spacing: PropTypes.oneOf([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]),

Going forward, you can use the theme to implement a custom Grid spacing transformation function: https://material-ui.com/system/spacing/#transformation.

Deprecation

theme.spacing.unit usage is deprecated, you can use the new API:

    [theme.breakpoints.up('sm')]: {
-     paddingTop: theme.spacing.unit * 12,
+     paddingTop: theme.spacing(12),
    },

Tip: you can provide more than 1 argument: theme.spacing(1, 2) // = '8px 16px'

@ifndefdeadmau5

This comment has been minimized.

@oliviertassinari oliviertassinari added this to the v4 milestone Jan 6, 2019
@oliviertassinari oliviertassinari changed the title [Grid] Apply theme.spacing.unit to grid spacing [Grid] Use a unitless spacing API Jan 6, 2019
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the visual differences intended? https://www.argos-ci.com/mui-org/material-ui/builds/40261

packages/material-ui/src/Grid/Grid.js Outdated Show resolved Hide resolved
packages/material-ui/src/Grid/Grid.d.ts Outdated Show resolved Hide resolved
@oliviertassinari oliviertassinari force-pushed the update-grid branch 2 times, most recently from 34ae605 to d5a7a81 Compare February 7, 2019 10:34
@oliviertassinari oliviertassinari force-pushed the update-grid branch 5 times, most recently from 3e685db to 0823684 Compare February 7, 2019 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants