[Box] doesn't apply maxWidth prop #25771

PetrShchukin · 2021-04-14T20:58:08Z

Box doesn't apply maxWidth prop

    <Box maxWidth="xs">
      Hello World
    </Box>

CodeSandBox:
https://codesandbox.io/s/hardcore-platform-tyfvw?file=/index.js:131-183

Mui version: v4.9.8 ✓

The text was updated successfully, but these errors were encountered:

oliviertassinari · 2021-04-14T21:28:54Z

@PetrShchukin What lead you to believe this API is supported?

PetrShchukin · 2021-04-15T10:17:27Z

In this page https://material-ui.com/components/box/#box, the latest sentence is "Any other properties supplied will be used by the style functions or spread to the root element." I'd expect maxWidth to be support as width is supported.

oliviertassinari · 2021-04-15T13:42:38Z

@PetrShchukin Ok thanks. I can't think of any way we could have solved the root cause, so I'm simply going to answer the direct question.

xs is a breakpoint value (and is 0px), it's not accepted as a final value. This would work

import React from "react";
import ReactDOM from "react-dom";
import Box from "@material-ui/core/Box";

function App() {
  return (
    <Box maxWidth={100} bgcolor="red">
      Hello World
    </Box>
  );
}

ReactDOM.render(<App />, document.querySelector("#app"));

https://codesandbox.io/s/quirky-moon-ss53u

In v5, you can also do:

import React from "react";
import ReactDOM from "react-dom";
import Box from "@material-ui/core/Box";

function App() {
  return (
    <Box maxWidth={(theme) => theme.breakpoints.values.sm} bgcolor="red">
      Hello World
    </Box>
  );
}

ReactDOM.render(<App />, document.querySelector("#app"));

https://codesandbox.io/s/agitated-agnesi-f5rfs

matheusmb · 2021-05-20T14:29:53Z

@oliviertassinari how to use your v5 example with Typescript? I tried here https://codesandbox.io/s/brave-pine-mhmf2?file=/index.tsx but I'm receiving the following TS Error:

  Overload 1 of 2, '(props: { component: ElementType<any>; } & { borderTop?: ResponsiveStyleValue<string | number | (string & {}) | MaxWidth<Key>[]>; borderRight?: ResponsiveStyleValue<...>; ... 62 more ...; textAlign?: ResponsiveStyleValue<...>; } & ... 4 more ... & Pick<...>): Element', gave the following error.
    Type '(theme: Theme) => number' is not assignable to type 'ResponsiveStyleValue<string | number | (string & {}) | MaxWidth<Key>[]>'.
      Type '(theme: Theme) => number' is not assignable to type '{ [key: string]: string | number | (string & {}) | MaxWidth<Key>[]; }'.
        Index signature is missing in type '(theme: Theme) => number'.
  Overload 2 of 2, '(props: DefaultComponentProps<BoxTypeMap<{}, "div">>): Element', gave the following error.
    Type '(theme: Theme) => number' is not assignable to type 'ResponsiveStyleValue<string | number | (string & {}) | MaxWidth<Key>[]>'.
      Type '(theme: Theme) => number' is not assignable to type '{ [key: string]: string | number | (string & {}) | MaxWidth<Key>[]; }'.ts(2769)
index.tsx(8, 19): Did you mean to call this expression?

Is the type definition correct?

It would be nice use the breakpoints tokens as strings, just as Chakra-UI has implemented, e.g.:

import React from "react";
import ReactDOM from "react-dom";
import Box from "@material-ui/core/Box";

function App() {
  return (
    <Box maxWidth="sm" bgcolor="red">
      Hello World
    </Box>
  );
}

ReactDOM.render(<App />, document.querySelector("#app"));

oliviertassinari · 2021-05-22T23:54:58Z

@matheusmb Thanks for raising the TypeScript issue. I can reproduce it too. @mnajdova should we fix it?

diff --git a/packages/material-ui-system/src/index.d.ts b/packages/material-ui-system/src/index.d.ts
index 6473f986fc..6d37237c87 100644
--- a/packages/material-ui-system/src/index.d.ts
+++ b/packages/material-ui-system/src/index.d.ts
@@ -4,7 +4,7 @@ import { CSSProperties } from './CSSProperties';
 import {
   OverwriteCSSProperties,
   AliasesCSSProperties,
-  StandardCSSProperties,
+  AllSystemCSSProperties,
 } from './styleFunctionSx';
 // disable automatic export
 export {};
@@ -291,36 +291,34 @@ export interface CSSOthersObjectForCSSObject {
   [propertiesName: string]: CSSInterpolation;
 }

-export type CustomSystemProps = OverwriteCSSProperties & AliasesCSSProperties;
+export interface CustomSystemProps extends AliasesCSSProperties, OverwriteCSSProperties {}

-export type SimpleSystemKeys = keyof Omit<
-  PropsFor<
-    ComposedStyleFunction<
-      [
-        typeof borders,
-        typeof display,
-        typeof flexbox,
-        typeof grid,
-        typeof palette,
-        typeof positions,
-        typeof shadows,
-        typeof sizing,
-        typeof spacing,
-        typeof typography,
-      ]
-    >
-  >,
-  keyof CustomSystemProps
+export type SimpleSystemKeys = keyof PropsFor<
+  ComposedStyleFunction<
+    [
+      typeof borders,
+      typeof display,
+      typeof flexbox,
+      typeof grid,
+      typeof palette,
+      typeof positions,
+      typeof shadows,
+      typeof sizing,
+      typeof spacing,
+      typeof typography,
+    ]
+  >
 >;

-// The SimpleSystemKeys are subset of the StandardCSSProperties, so this should be ok
-// This is needed as these are used as keys inside StandardCSSProperties
-type StandardSystemKeys = Extract<SimpleSystemKeys, keyof StandardCSSProperties>;
+// The SimpleSystemKeys are subset of the AllSystemCSSProperties, so this should be ok
+// This is needed as these are used as keys inside AllSystemCSSProperties
+type StandardSystemKeys = Extract<SimpleSystemKeys, keyof AllSystemCSSProperties>;

-export type SystemProps = {
-  [K in StandardSystemKeys]?: ResponsiveStyleValue<StandardCSSProperties[K]>;
-} &
-  CustomSystemProps;
+export type SystemProps<Theme extends object = {}> = {
+  [K in StandardSystemKeys]?:
+    | ResponsiveStyleValue<AllSystemCSSProperties[K]>
+    | ((theme: Theme) => ResponsiveStyleValue<AllSystemCSSProperties[K]>);
+};

 export {
   default as unstable_styleFunctionSx,
diff --git a/packages/material-ui-system/src/styleFunctionSx/styleFunctionSx.d.ts b/packages/material-ui-system/src/styleFunctionSx/styleFunctionSx.
d.ts
index 2a30a79bcc..5800fbedc3 100644
--- a/packages/material-ui-system/src/styleFunctionSx/styleFunctionSx.d.ts
+++ b/packages/material-ui-system/src/styleFunctionSx/styleFunctionSx.d.ts
@@ -26,13 +26,13 @@ export interface CSSSelectorObject<Theme extends object = {}> {

 /**
  * Map of all available CSS properties (including aliases) and their raw value.
- * Only used internally to map CCS properties to input types (responsive value,
+ * Only used internally to map CSS properties to input types (responsive value,
  * theme function or nested) in `SystemCssProperties`.
  */
 export interface AllSystemCSSProperties
   extends Omit<StandardCSSProperties, keyof OverwriteCSSProperties>,
-    AliasesCSSProperties,
-    OverwriteCSSProperties {}
+    OverwriteCSSProperties,
+    AliasesCSSProperties {}

 export type SystemCssProperties<Theme extends object = {}> = {
   [K in keyof AllSystemCSSProperties]:
diff --git a/packages/material-ui/src/Box/Box.d.ts b/packages/material-ui/src/Box/Box.d.ts
index d9c41651a2..f4a5104ede 100644
--- a/packages/material-ui/src/Box/Box.d.ts
+++ b/packages/material-ui/src/Box/Box.d.ts
@@ -5,7 +5,7 @@ import { Theme } from '../styles/createTheme';

 export interface BoxTypeMap<P = {}, D extends React.ElementType = 'div'> {
   props: P &
-    SystemProps & {
+    SystemProps<Theme> & {
       children?: React.ReactNode;
       component?: React.ElementType;
       ref?: React.Ref<unknown>;
diff --git a/packages/material-ui/src/Grid/Grid.d.ts b/packages/material-ui/src/Grid/Grid.d.ts
index f7e35fbc2f..4a5b602225 100644
--- a/packages/material-ui/src/Grid/Grid.d.ts
+++ b/packages/material-ui/src/Grid/Grid.d.ts
@@ -14,7 +14,7 @@ export type GridSize = 'auto' | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12

 export interface GridTypeMap<P = {}, D extends React.ElementType = 'div'> {
   props: P &
-    SystemProps & {
+    SystemProps<Theme> & {
       /**
        * The content of the component.
        */
diff --git a/packages/material-ui/src/Stack/Stack.d.ts b/packages/material-ui/src/Stack/Stack.d.ts
index 1703f2a4b5..e65d46be71 100644
--- a/packages/material-ui/src/Stack/Stack.d.ts
+++ b/packages/material-ui/src/Stack/Stack.d.ts
@@ -5,7 +5,7 @@ import { Theme } from '../styles/createTheme';

 export interface StackTypeMap<P = {}, D extends React.ElementType = 'div'> {
   props: P &
-    SystemProps & {
+    SystemProps<Theme> & {
       ref?: React.Ref<unknown>;
       /**
        * The content of the component.
diff --git a/packages/material-ui/src/Typography/Typography.d.ts b/packages/material-ui/src/Typography/Typography.d.ts
index eecbc311fb..721e634f5b 100644
--- a/packages/material-ui/src/Typography/Typography.d.ts
+++ b/packages/material-ui/src/Typography/Typography.d.ts
@@ -10,7 +10,7 @@ export interface TypographyPropsVariantOverrides {}

 export interface TypographyTypeMap<P = {}, D extends React.ElementType = 'span'> {
   props: P &
-    SystemProps & {
+    SystemProps<Theme> & {
       /**
        * Set the text-align on the component.
        * @default 'inherit'

Regarding native support of the breakpoint values. It could make sense. We have these props on the Dialog and Container components. We would need to do:

diff --git a/packages/material-ui-system/src/sizing.js b/packages/material-ui-system/src/sizing.js
index f957b440be..f9227c74fe 100644
--- a/packages/material-ui-system/src/sizing.js
+++ b/packages/material-ui-system/src/sizing.js
@@ -1,5 +1,6 @@
 import style from './style';
 import compose from './compose';
+import { handleBreakpoints } from './breakpoints';

 function transform(value) {
   return value <= 1 ? `${value * 100}%` : value;
@@ -10,10 +11,20 @@ export const width = style({
   transform,
 });

-export const maxWidth = style({
-  prop: 'maxWidth',
-  transform,
-});
+export const maxWidth = (props) => {
+  if (props.maxWidth) {
+    const styleFromPropValue = (propValue) => {
+      const breakpoint = props.theme.breakpoints.values[propValue];
+      return {
+        maxWidth: breakpoint || transform(propValue),
+      };
+    }
+    return handleBreakpoints(props, props.maxWidth, styleFromPropValue);
+  }
+  return null;
+};
+
+maxWidth.filterProps = ['maxWidth'];

 export const minWidth = style({
   prop: 'minWidth',

to have

import React from 'react';
import Box from '@material-ui/core/Box';

export default function App() {
  return (
    <Box maxWidth="sm" bgcolor="red" m={[1, 2]}>
      Hello World
    </Box>
  );
}

it's for instance supported in Tailwind: https://tailwindcss.com/docs/max-width.

mnajdova · 2021-05-27T14:31:39Z

I like this idea. Using the breakpoints as values makes sense 👍

ansh-saini · 2021-06-20T11:28:54Z

@oliviertassinari Can I work on this?

ansh-saini · 2021-06-20T15:25:53Z

Hey @oliviertassinari I opened a PR which only fixes the TypeScript issue.
I have not implemented the tailwind kind of functionality on the maxWidth prop. Should I implement it in a different PR?

oliviertassinari · 2021-06-20T15:54:00Z

Should I implement it in a different PR?

@ansh-saini Doing it in a second PR sounds great.

ansh-saini · 2021-06-20T16:04:20Z

Alright, thanks.

PetrShchukin added the status: needs triage label Apr 14, 2021

oliviertassinari added component: Box status: incomplete and removed status: needs triage labels Apr 14, 2021

oliviertassinari self-assigned this Apr 15, 2021

oliviertassinari closed this Apr 15, 2021

oliviertassinari added question and removed status: incomplete labels Apr 15, 2021

oliviertassinari changed the title ~~Box doesn't apply maxWidth prop~~ [Box] doesn't apply maxWidth prop Apr 15, 2021

oliviertassinari added design system package: system and removed question design system labels May 22, 2021

oliviertassinari reopened this May 22, 2021

oliviertassinari added the good first issue label May 26, 2021

ansh-saini mentioned this issue Jun 20, 2021

[Box] Fix TypeScript error on maxWidth prop #26859

Merged

1 task done

ansh-saini linked a pull request that will close this issue Jun 27, 2021

[Box] Add breakpoint value support to maxWidth prop #26984

Open

1 task done

Jun	JUL	Aug
	07
2020	2021	2022

mui-org / material-ui

[Box] doesn't apply maxWidth prop #25771

[Box] doesn't apply maxWidth prop #25771

PetrShchukin commented Apr 14, 2021

oliviertassinari commented Apr 14, 2021

PetrShchukin commented Apr 15, 2021 •

edited

oliviertassinari commented Apr 15, 2021 •

edited

matheusmb commented May 20, 2021 •

edited

oliviertassinari commented May 22, 2021 •

edited

mnajdova commented May 27, 2021

ansh-saini commented Jun 20, 2021

ansh-saini commented Jun 20, 2021

oliviertassinari commented Jun 20, 2021

ansh-saini commented Jun 20, 2021

mui-org / material-ui

Sponsor mui-org/material-ui

[Box] doesn't apply maxWidth prop #25771

[Box] doesn't apply maxWidth prop #25771

Comments

PetrShchukin commented Apr 14, 2021

oliviertassinari commented Apr 14, 2021

PetrShchukin commented Apr 15, 2021 • edited

oliviertassinari commented Apr 15, 2021 • edited

matheusmb commented May 20, 2021 • edited

oliviertassinari commented May 22, 2021 • edited

mnajdova commented May 27, 2021

ansh-saini commented Jun 20, 2021

ansh-saini commented Jun 20, 2021

oliviertassinari commented Jun 20, 2021

ansh-saini commented Jun 20, 2021

PetrShchukin commented Apr 15, 2021 •

edited

oliviertassinari commented Apr 15, 2021 •

edited

matheusmb commented May 20, 2021 •

edited

oliviertassinari commented May 22, 2021 •

edited