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

[material-ui][Stepper] Generate class for nonLinear prop #42620

Merged
merged 11 commits into from
Jun 18, 2024
Merged

[material-ui][Stepper] Generate class for nonLinear prop #42620

merged 11 commits into from
Jun 18, 2024

Conversation

alexismo
Copy link
Contributor

@alexismo alexismo commented Jun 11, 2024

This PR adds the nonLinear utility class to the Stepper component when the nonLinear prop is used.

Deploy preview: https://deploy-preview-42620--material-ui.netlify.app/material-ui/react-stepper/#non-linear

Closes #42360

@zannager zannager added the component: stepper This is the name of the generic UI component, not the React module! label Jun 12, 2024
@zannager zannager requested a review from DiegoAndai June 12, 2024 15:00
@sai6855 sai6855 closed this Jun 13, 2024
@sai6855 sai6855 reopened this Jun 13, 2024
@sai6855
Copy link
Contributor

sai6855 commented Jun 13, 2024

@alexismo

  1. can you add test case for change you introduced in this file https://github.com/mui/material-ui/blob/next/packages/mui-material/src/Stepper/Stepper.test.tsx. Refer to existing test cases on how to write a test
  2. Also run pnpm docs:api to generate docs for new class

@@ -5,7 +5,7 @@ import Step, { StepProps, stepClasses } from '@mui/material/Step';
import StepLabel from '@mui/material/StepLabel';
import StepConnector, { stepConnectorClasses } from '@mui/material/StepConnector';
import StepContent, { stepContentClasses } from '@mui/material/StepContent';
import Stepper, { stepperClasses as classes } from '@mui/material/Stepper';
import Stepper, { stepperClasses as classes, stepperClasses } from '@mui/material/Stepper';
Copy link
Contributor

Choose a reason for hiding this comment

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

stepperClasses is already imported and named as classes, so you can use classes instead of importing stepperClasses again

Comment on lines 274 to 276
setProps({ alternativeLabel: true });

expect(stepper).to.have.class(classes.nonLinear)
Copy link
Contributor

@sai6855 sai6855 Jun 13, 2024

Choose a reason for hiding this comment

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

i believe these are not required, just testing presence of nonLinear class is required, which is what line 272 is doing.

per PR review

Signed-off-by: Alexis Morin <the.alexis.morin@gmail.com>
@sai6855 sai6855 closed this Jun 13, 2024
@sai6855 sai6855 reopened this Jun 13, 2024
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Hey @alexismo, thanks for working on this. Also thanks @sai6855 for the review.

The addition of the class looks good. For consistency, may I ask you to also add the nonLinear key to the MuiStepper styleOverrides? You should be able to follow how the alternativeLabel key is added here.

To confirm that the requested functionality is supported, this is what you are looking for, right?: codesandbox

Screenshot 2024-06-14 at 10 52 50

Signed-off-by: Alexis Morin <the.alexis.morin@gmail.com>
@alexismo
Copy link
Contributor Author

alexismo commented Jun 14, 2024

Thank you @DiegoAndai for the review 🙏 The example you're showing in CSB looks correct.

Here it is with a styleOverrides applied to nonLinear: CodeSandBox with nonLinear styleOverride

@ZeeshanTamboli ZeeshanTamboli added package: material-ui Specific to @mui/material enhancement This is not a bug, nor a new feature needs cherry-pick The PR should be cherry-picked to master after merge labels Jun 16, 2024
@mui-bot
Copy link

mui-bot commented Jun 18, 2024

Netlify deploy preview

https://deploy-preview-42620--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 19eb8c7

Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

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

Thanks @alexismo for the pull request. I pushed two commits to update the test description and apply prettier. Thanks to @sai6855 and @DiegoAndai for the reviews. I believe we can merge this now and also to the master branch for v5.

@ZeeshanTamboli ZeeshanTamboli changed the title [material-ui][Stepper] Generate class for nonLinear prop [material-ui][Stepper] Generate class for nonLinear prop Jun 18, 2024
@ZeeshanTamboli ZeeshanTamboli merged commit ee4e109 into mui:next Jun 18, 2024
22 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 18, 2024
Signed-off-by: Alexis Morin <the.alexis.morin@gmail.com>
Co-authored-by: ZeeshanTamboli <zeeshan.tamboli@gmail.com>
joserodolfofreitas pushed a commit to joserodolfofreitas/material-ui that referenced this pull request Jul 29, 2024
Signed-off-by: Alexis Morin <the.alexis.morin@gmail.com>
Co-authored-by: ZeeshanTamboli <zeeshan.tamboli@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: stepper This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature needs cherry-pick The PR should be cherry-picked to master after merge package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][Stepper] Not generating a utility class for nonLinear
6 participants