Skip to content

Conversation

@mnajdova
Copy link
Member

@mnajdova mnajdova commented Sep 22, 2020

Breaking changes

  • Breakpoints are now treated as values instead of ranges.
    The behavior of down(key) was changed to define media query less than the value defined with the corresponding breakpoint (exclusive).
    The behavior of between(start, end) was also updated to define media query for the values between the actual values of start (inclusive) and end (exclusive).

    Find examples of the changes required defined below:

-theme.breakpoints.down('sm') // '@media (max-width:959.95px)' - [0, sm + 1) => [0, md)
+theme.breakpoints.down('md') // '@media (max-width:959.95px)' - [0, md)
-theme.breakpoints.between('sm', 'md') // '@media (min-width:600px) and (max-width:1279.95px)' - [sm, md + 1) => [sm, lg)
+theme.breakpoints.between('sm', 'lg') // '@media (min-width:600px) and (max-width:1279.95px)' - [sm, lg)

There are also usages of breakpoints.down() & breakpoints.between() in Dialog and HiddenCss, but I believe these should not be changed. The reasons are - the Dialog invokes the down() method with number which behavior is not changed; HiddenCss is following the new convention.

### Changes required on the adaptV4theme

Seems like this is the first change that will actually require implementing createMuiThemeV4 utility instead of adapter, as we need to change the theme after it has been created. Should we start with this change as part of this PR? I would say yes if we want to fully say, we will support the v4 theme structure under deprecation in v5

Closes #13448
Part of #20012

@mui-pr-bot
Copy link

mui-pr-bot commented Sep 22, 2020

Details of bundle changes

Generated by 🚫 dangerJS against c3ae83e

@mnajdova mnajdova marked this pull request as ready for review September 22, 2020 14:50
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

@oliviertassinari oliviertassinari added the breaking change Introduces changes that are not backward compatible. label Sep 22, 2020
@mnajdova
Copy link
Member Author

Does it mean we can kill this block?
image
http://0.0.0.0:3000/customization/breakpoints/#default-breakpoints

@oliviertassinari good catch! Removed 👍

Regarding supporting the changes in the adapter:

Changes required on the adaptV4theme

Seems like this is the first change that will actually require implementing createMuiv4Theme utility instead of adapter, as we need to change the theme after it has been created. Should we start with this change as part of this PR? I would say yes if we want to fully say, we will support the v4 theme structure under deprecation in v5

Should I start working on createMuiv4Theme, or should we disregard these changes from the adaptV4Theme ?

@oliviertassinari
Copy link
Member

Should I start working on createMuiv4Theme, or should we disregard these changes from the adaptV4Theme ?

I guess we could do it in a follow up pull request? Right after this one?

@oliviertassinari
Copy link
Member

Something is off with the Hidden component. We didn't handle it. For instance, open https://next--material-ui.netlify.app/getting-started/installation/ with a screen width of 1000px.

@oliviertassinari
Copy link
Member

On the css implementation side:

diff --git a/docs/src/modules/components/AppDrawer.js b/docs/src/modules/components/AppDrawer.js
index 1e5212d26b..c0ee761c19 100644
--- a/docs/src/modules/components/AppDrawer.js
+++ b/docs/src/modules/components/AppDrawer.js
@@ -192,7 +192,7 @@ function AppDrawer(props) {
         </SwipeableDrawer>
       </Hidden>
       {disablePermanent ? null : (
-        <Hidden mdDown implementation="css">
+        <Hidden lgDown implementation="css">
           <Drawer
             classes={{
               paper: classes.paper,
diff --git a/docs/src/pages/components/drawers/ResponsiveDrawer.tsx b/docs/src/pages/components/drawers/ResponsiveDrawer.tsx
index 4e744234bf..ce84ee313f 100644
--- a/docs/src/pages/components/drawers/ResponsiveDrawer.tsx
+++ b/docs/src/pages/components/drawers/ResponsiveDrawer.tsx
@@ -145,7 +145,7 @@ export default function ResponsiveDrawer(props: Props) {
             {drawer}
           </Drawer>
         </Hidden>
-        <Hidden xsDown implementation="css">
+        <Hidden smDown implementation="css">
           <Drawer
             classes={{
               paper: classes.drawerPaper,
diff --git a/docs/src/pages/getting-started/templates/blog/FeaturedPost.tsx b/docs/src/pages/getting-started/templates/blog/FeaturedPost.tsx
index 3d1a2d42b0..a3cb23b99b 100644
--- a/docs/src/pages/getting-started/templates/blog/FeaturedPost.tsx
+++ b/docs/src/pages/getting-started/templates/blog/FeaturedPost.tsx
@@ -54,7 +54,7 @@ export default function FeaturedPost(props: FeaturedPostProps) {
               </Typography>
             </CardContent>
           </div>
-          <Hidden xsDown>
+          <Hidden smDown>
             <CardMedia
               className={classes.cardMedia}
               image={post.image}
diff --git a/docs/src/pages/premium-themes/onepirate/modules/views/ProductCTA.tsx b/docs/src/pages/premium-themes/onepirate/modules/views/ProductCTA.tsx
index 636f80cb2e..7b2bc3c844 100644
--- a/docs/src/pages/premium-themes/onepirate/modules/views/ProductCTA.tsx
+++ b/docs/src/pages/premium-themes/onepirate/modules/views/ProductCTA.tsx
@@ -105,7 +105,7 @@ function ProductCTA(props: WithStyles<typeof styles>) {
           </div>
         </Grid>
         <Grid item xs={12} md={6} className={classes.imagesWrapper}>
-          <Hidden smDown>
+          <Hidden mdDown>
             <div className={classes.imageDots} />
             <img
               src="https://images.unsplash.com/photo-1527853787696-f7be74f2e39a?auto=format&fit=crop&w=750&q=80"
diff --git a/docs/src/pages/premium-themes/paperbase/Paperbase.tsx b/docs/src/pages/premium-themes/paperbase/Paperbase.tsx
index 767d406e05..92deb0f38d 100644
--- a/docs/src/pages/premium-themes/paperbase/Paperbase.tsx
+++ b/docs/src/pages/premium-themes/paperbase/Paperbase.tsx
@@ -210,7 +210,7 @@ function Paperbase(props: PaperbaseProps) {
               onClose={handleDrawerToggle}
             />
           </Hidden>
-          <Hidden xsDown implementation="css">
+          <Hidden smDown implementation="css">
             <Navigator PaperProps={{ style: { width: drawerWidth } }} />
           </Hidden>
         </nav>

@oliviertassinari
Copy link
Member

on the js implementation, maybe?

diff --git a/packages/material-ui/src/withWidth/withWidth.js b/packages/material-ui/src/withWidth/withWidth.js
index 96fec3f6e3..84a5af2255 100644
--- a/packages/material-ui/src/withWidth/withWidth.js
+++ b/packages/material-ui/src/withWidth/withWidth.js
@@ -17,7 +17,7 @@ export const isWidthUp = (breakpoint, width, inclusive = true) => {
 };

 // By default, returns true if screen width is the same or less than the given breakpoint.
-export const isWidthDown = (breakpoint, width, inclusive = true) => {
+export const isWidthDown = (breakpoint, width, inclusive = false) => {
   if (inclusive) {
     return breakpointKeys.indexOf(width) <= breakpointKeys.indexOf(breakpoint);
   }

@ghost
Copy link

ghost commented Oct 11, 2020

The major fix for this break change is between("xs", "sm"), to replace down("xs"), right? Is there more?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Introduces changes that are not backward compatible.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Breakpoints] functions down() and between() adds +1 to index

4 participants