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

[system] spacing not giving warning that theme.spacing array cannot be combined with non integer value #23187

Closed
1 task
mnajdova opened this issue Oct 21, 2020 · 0 comments · Fixed by #23460
Closed
1 task
Assignees
Labels
docs Improvements or additions to the documentation package: system Specific to @mui/system

Comments

@mnajdova
Copy link
Member

mnajdova commented Oct 21, 2020

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

When there is array of values specified for the spacing inside the theme, the spacing system function returns unexpected value, without adding any warning that this isn't supported.

We could probably fix this by making this test pass:

 it('should error dev that array values for the theme.spacing cannot be combined with non integer values for the system', () => {
    let output;

    expect(() => {
      output = spacing({
        theme: {
          spacing: [1, 2, 3, 4, 5, 6],
        },
        p: 0.5,
      });
    }).toErrorDev(
      'Material-UI: The `theme.spacing` array type cannot be combined with non integer values.\n' +
        'You should either use an integer value that can be used as index, or define the `theme.spacing` as a number.',
    );

    expect(output).to.deep.equal({ padding: undefined });
});

Examples 🌈

This is one example of non valid usage

 const output = spacing({
   theme: {
     spacing: [1, 2, 3, 4, 5, 6],
   },
   p: 0.5,
 });

Motivation 🔦

This will explicitly ensure that we are thinking about all invalid cases that can be used, and can guide the developers towards the correct approach.

First reported on #23053 (comment)

@mnajdova mnajdova added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 21, 2020
@mnajdova mnajdova self-assigned this Oct 21, 2020
@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation package: system Specific to @mui/system and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 21, 2020
@mnajdova mnajdova mentioned this issue Oct 21, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation package: system Specific to @mui/system
Projects
None yet
2 participants