Skip to content

[Stepper] Remove Paper and built-in padding#22564

Merged
mbrookes merged 12 commits into
mui:nextfrom
mbrookes:stepper-remove-paper
Sep 14, 2020
Merged

[Stepper] Remove Paper and built-in padding#22564
mbrookes merged 12 commits into
mui:nextfrom
mbrookes:stepper-remove-paper

Conversation

@mbrookes

@mbrookes mbrookes commented Sep 11, 2020

Copy link
Copy Markdown
Member

Breaking changes

  • [Stepper] The root component (Paper) was replaced with a <div>. Stepper no longer has elevation, nor inherits Paper's props.
-<Stepper elevation={2}>
-  <Step>
-    <StepLabel>Hello world</StepLabel>
-  </Step>
-</Stepper>
+<Paper square elevation={2}>
+  <Stepper>
+    <Step>
+      <StepLabel>Hello world</StepLabel>
+    </Step>
+  </Stepper>
+<Paper>
  • [Stepper] Remove the built-in 24px padding.
-<Stepper>
-  <Step>
-    <StepLabel>Hello world</StepLabel>
-  </Step>
-</Stepper>
+<Stepper style={{ padding: 24 }}>
+  <Step>
+    <StepLabel>Hello world</StepLabel>
+  </Step>
+</Stepper>

Closes #18423

@mbrookes mbrookes added scope: stepper Changes related to the stepper. breaking change Introduces changes that are not backward compatible. labels Sep 11, 2020

@oliviertassinari oliviertassinari left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that with the change, we can use the default outlined container for the demos, instead of the grey background.

Comment thread docs/src/pages/guides/migration-v4/migration-v4.md Outdated
Comment thread docs/src/pages/guides/migration-v4/migration-v4.md Outdated
@mui-pr-bot

mui-pr-bot commented Sep 11, 2020

Copy link
Copy Markdown

Details of bundle changes

Generated by 🚫 dangerJS against 09a3cf2

@oliviertassinari oliviertassinari left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The mobile stepper still has a white background, it's easier to spot with:

Comment thread docs/src/pages/components/steppers/steppers.md Outdated
Comment thread docs/src/pages/components/steppers/steppers.md Outdated
Comment thread docs/src/pages/components/steppers/steppers.md Outdated
Comment thread docs/src/pages/components/steppers/steppers.md Outdated
Co-authored-by: Olivier Tassinari <[email protected]>
@mbrookes

mbrookes commented Sep 11, 2020

Copy link
Copy Markdown
Member Author

@oliviertassinari I've tweaked the Button layout and variant for the first demo to better match the original component:

Spec:
https://material.io/archive/guidelines/components/steppers.html#steppers-types-of-steppers
image

Before:
image

After:
image

Once it's good, I'll do the rest of the demos.

@oliviertassinari

Copy link
Copy Markdown
Member

@mbrookes Looks better 👍

@mbrookes

Copy link
Copy Markdown
Member Author

Option 2 – add a white background so you can distinguish the component boundaries from the demo container padding, add a divider:

image

@oliviertassinari

oliviertassinari commented Sep 11, 2020

Copy link
Copy Markdown
Member

@mbrookes I think that option 1 is better. It will make the source of the demo simpler. Also, I don't think that the stepper should have any padding.

@oliviertassinari

oliviertassinari commented Sep 11, 2020

Copy link
Copy Markdown
Member

I had to override the padding in the store:

Capture d’écran 2020-09-12 à 00 21 26

And if you are ShutterStock (yes the store is inspired by shutterstock.com & themes.getbootstrap.com), you would need too:

Capture d’écran 2020-09-12 à 00 22 35


What about we removing this padding?

@mbrookes

Copy link
Copy Markdown
Member Author

I don't think that the stepper should have any padding.

I agree. I just didn't want to be too radical!

Mk 3:

image

@oliviertassinari

oliviertassinari commented Sep 12, 2020

Copy link
Copy Markdown
Member

I agree. I just didn't want to be too radical!

Oh right, one breaking change at the time, removal of padding could be the very next one 👼.

Mk 3:

A couple of thoughts:

  1. Having the buttons closer to each other makes it easier to use the playground. Otherwise, you need the mouse to make more movement to go back.
  2. Maybe we should have one static demos of the Stepper at the top of the page with an inline preview that showcase the composition API. The demos are quite long and can be hard to navigate.
  3. For the customized stepper, I think that we can drop the buttons, make is static, and have step 2 as active: https://deploy-preview-22564--material-ui.netlify.app/components/steppers/#customized-stepper. It would cover all the use cases and simplify the demo.
  4. Do we need the labels like "This is the bit I really care about!", What if we already displayed the active step? e.g. "Step 2" => simpler demos.
  5. Removing the built-in padding of the Stepper would allow to simply the demos, as would allow to remove the padding we add on the controls.

@mbrookes

mbrookes commented Sep 12, 2020

Copy link
Copy Markdown
Member Author

We had a power cut this afternoon, but have been working offline on simplifying the demos. It seems I've already hit on several of the things you suggested. Take a look.

Having the buttons closer to each other makes it easier to use the playground. Otherwise, you need the mouse to make more movement to go back.

That's down to the demo not having a container. You can't have it both ways. 😄
You don't really need to use the back button to navigate the demos.

Maybe we should have one static demos of the Stepper at the top of the page with an inline preview that showcase the composition API.

I think we need to show a functioning Stepper first.

The demos are quite long and can be hard to navigate.

Most of the demos are now simplified.

For the customized stepper, I think that we can drop the buttons

Already done.

What if we already displayed the active step? "Step 2" => simpler demos

Already simpler, but I'll take another look.

Removing the built-in padding of the Stepper

Already done in "Mk 3".

would allow to remove the padding we add on the controls.

Already done.

@oliviertassinari oliviertassinari changed the title [Stepper] Remove Paper [Stepper] Remove Paper and built-in padding Sep 13, 2020
@mbrookes
mbrookes merged commit f7d4af9 into mui:next Sep 14, 2020
@mbrookes
mbrookes deleted the stepper-remove-paper branch September 14, 2020 19:47
@oliviertassinari oliviertassinari added this to the v5 milestone Sep 15, 2020
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. scope: stepper Changes related to the stepper.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Stepper] Remove Paper component

3 participants