[Grid] grid item doesn't respond to grid container's responsive columns correctly

[kkorach]

• I have followed (at least) the PR section of the contributing guide.

Closes #29379

Problem:

• Consider <Grid item xs={1} />; the 1 column prop does not carry over to the other breakpoints.

• In generateGrid the first few lines are

  const size = ownerState[breakpoint];

  if (!size) {
    return;
  }

• In my case where I only specify xs={1} there won't be any breakpoints in ownerState, so it will exit out of the method.

As a result, the following two show different behaviours; the first one shows the correct behaviour;
codesandbox

    <Grid container spacing={1} columns={{ xs: 4, sm: 8, md: 12 }}>
      {Array.from(Array(16)).map((_, index) => (
        <Grid item xs={1} sm={1} md={1} key={index}>
          <Item>xs=2</Item>
        </Grid>
      ))}
    </Grid>

codesandbox

    <Grid container spacing={1} columns={{ xs: 4, sm: 8, md: 12 }}>
      {Array.from(Array(16)).map((_, index) => (
        <Grid item xs={1} key={index}>
          <Item>xs=2</Item>
        </Grid>
      ))}
    </Grid>

[kkorach]

@hbjORbj Ready for a review

[mui-pr-bot]

Details of bundle changes

Generated by 🚫 dangerJS against 302fbcc

[kkorach]

So the first big change was to convert this method to match generateDirection and the other generate methods. This allowed me to remember the size from the previously defined breakpoint. So if xs={1} is specified and not other breakpoints, each breakpoint will get size 1. By doing it here, it won't affect ownerState where we would want sm=false if sm is not specified.

[hbjORbj]

@kkorach Hey, thanks for your contribution. I edited the description to describe clearly what problem we are solving in this PR. I will review your code in more detail before the end of the day :)

[hbjORbj]

@kkorach I can confirm that the issue is fixed here

Can you (1) revert the changes to demos and (2) add a test verifying that your changes fix the issue?

[kkorach]

@hbjORbj Made the changes to Grid. I also had to fix a bug in resolveBreakpointValues. It would set the previous variable every time through the loop, regardless of whether there was a value in breakpointValues. This meant that if you passed in xs, sm, md, it would work up to lg. The pass through with xl would fail because breakpointsValue[lg] is undefined. I've changed the logic to only set previous if breakpointValues[breakpoint/i] has a value. It does make the code a little more messy.

[kkorach]

What are the next steps, is there anything I need to do? @hbjORbj

[hbjORbj]

@kkorach Thank you for your amazing contribution. I will merge this after one more review from another teammate.

[hbjORbj]

@siriwatknp Argos tests are failing, but as you can see in the codesandboxes below, the two demos (before and after changes of this PR) are actually identical. Do you know why this is happening?

Before:

• https://codesandbox.io/s/stupefied-driscoll-5e01m
  After:
• https://codesandbox.io/s/vigorous-northcutt-5lpvw?file=/src/Demo.tsx

[siriwatknp]

@siriwatknp Argos tests are failing, but as you can see in the codesandboxes below, the two demos (before and after changes of this PR) are actually identical. Do you know why this is happening?

Before:

• https://codesandbox.io/s/stupefied-driscoll-5e01m
  After:
• https://codesandbox.io/s/vigorous-northcutt-5lpvw?file=/src/Demo.tsx

Can you try updating the branch with the latest master?

[siriwatknp]

I will merge this after one more review from another teammate.

@hbjORbj I might not have time to review it in detail. From your point of view, do you see any tests that might be needed?

[hbjORbj]

@kkorach I had a more careful look and I was able to detect unnecessary changes. (1) I reverted your changes in resolveBreakpointValues function. (2) I reverted your changes in Grid that always produce a base of{ xs: true, sm: true, md: true, lg: true, xl: true }. We don't need to include breakpoints that were not provided by user in columns in base. (3) I like how you refactored generateGrid so that we can simply put the function along with other generate...**. (4) I made some additions to code that address the original issue. I can confirm in this codesandbox that the issue has been fixed.

@siriwatknp I added one more test. FYI, there is much less overall change now, so you should be able to review much more easily.

[hbjORbj]

@mnajdova @siriwatknp

Update: Ignore this. I was wrong.

I think we should approve the Argos failure. Apparently, Grid used in PickersToolbar was not working properly before the change of this PR. More specifically, justify-content: space-between was not being applied previously. By looking at code, the expected behaviour is having this property.

<PickersToolbarGrid
  container
  justifyContent="space-between"
  className={classes.dateTitleContainer}
  direction={isLandscape ? landscapeDirection : 'row'}
  alignItems={isLandscape ? 'flex-start' : 'flex-end'}
>

Before the change of this PR:

After the change of this PR:

[mnajdova]

I haven't looked too deep here, but I don't understand how the changes of the PR are related to using the justifyContent property. It kind of scares me that it is introducing a braking change. I may be wrong. Regarding the new look of the DatePicker I don't have strong opinion, I would ask @danilo-leal to take a look at it.

[hbjORbj]

@mnajdova I was wrong about why the argos was failing. Now argos successfully passes. In conclusion, there are only small changes in code. I commented the two places of change. The issue that this PR was originally trying to solve is fixed and can be seen here.

@danilo-leal No need to worry about changed demos!

[hbjORbj]

This PR changed the scope of this variable size. As a result, the value of previous breakpoint can be used now.

[hbjORbj]

Another addition of this PR. This prevents width in the next line from being NaN.

[siriwatknp]

👍 LGTM