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

[StepConnector] state styling doesn't work with 'Alternative Label' #13104

Closed
2 tasks done
jmaloon opened this issue Oct 5, 2018 · 4 comments
Closed
2 tasks done

[StepConnector] state styling doesn't work with 'Alternative Label' #13104

jmaloon opened this issue Oct 5, 2018 · 4 comments
Labels
component: stepper This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@jmaloon
Copy link
Contributor

jmaloon commented Oct 5, 2018

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

Expected Behavior

StepConnector should receive 'state' classes (eg 'active', 'completed') based on the status of the parent Step

Current Behavior

The 'state' classes are not applied to the StepConnector when the Stepper uses the 'alternativeLabel' prop...but remove the 'alternativeLabel' and voila - it works!

Steps to Reproduce

  1. https://codesandbox.io/s/64k4mrz1qz

Context

Your Environment

Tech Version
Material-UI v1.?.?
React
Browser
TypeScript
etc.
@eps1lon
Copy link
Member

eps1lon commented Oct 5, 2018

Nice find. The hole logic and markup is completely changed when using alternativeLabel. I wonder if it isn't simpler to wrap the Step and Label into one container and change flex-direction depending on alternativeLabel.

@eps1lon eps1lon closed this as completed Oct 5, 2018
@eps1lon eps1lon reopened this Oct 5, 2018
@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. component: stepper This is the name of the generic UI component, not the React module! labels Oct 5, 2018
@oliviertassinari
Copy link
Member

@jmaloon We should be able to address this issue the same way #13023 was solved. In the Step component, we can forward the state properties. Do you want to work on it?

@eps1lon I doubt we can make the implementation any simpler, but it's always a good idea to give it a try based on the intuition we can.

@jmaloon
Copy link
Contributor Author

jmaloon commented Oct 5, 2018

@oliviertassinari thanks for chiming in.
Sure I would love to contribute, been using the library for some time and really impressed. I'll create a PR.

@jmaloon
Copy link
Contributor Author

jmaloon commented Oct 6, 2018

@oliviertassinari @eps1lon 'alternativeLabel' renders the Connector after the step, whereas a normal stepper renders the Connector before the step. Therefore to achieve a consistent styling with the baseline Stepper I pass the 'prevStepState' to the 'alternativeLabel' Connector.

I made the changes in PR #13130. I didn't quite figure out how to run all the tests so I didn't add any test cases. Let me know your thoughts. Thanks.

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! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

No branches or pull requests

3 participants