-
Notifications
You must be signed in to change notification settings - Fork 6
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
Stepper component #675
base: release/1.1.0
Are you sure you want to change the base?
Stepper component #675
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## release/1.1.0 #675 +/- ##
===============================================
Coverage 100.00% 100.00%
===============================================
Files 89 94 +5
Lines 1663 1727 +64
Branches 584 599 +15
===============================================
+ Hits 1663 1727 +64 ☔ View full report in Codecov by Sentry. |
@byohannes please can you change how the component is being rendered. Please can you make it so that the header and the content render separately As you can see from the angular material example the header and content are separate which then gives us greater control on how the component can look |
@alexwbbr, thanks for the review; I have updated the component to reflect the above-requested changes, and now each step does have a separate header and content as shown below |
I have implemented the required changes @alexwbbr on the latest changes as shown on the attached images: |
… class based demos to look alike material UI
…ial UI design requirement and improve demos
01cf538
to
612d999
Compare
'aria-selected': index === activeStep ? 'true' : 'false', | ||
'aria-posinset': index + 1, | ||
'aria-setsize': Children.count(children), | ||
tabIndex: index === activeStep ? '0' : '-1', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove this line, if the user can click on a step they are not on and jump to it then a user also needs to be able to tab to that step
stepperClassName="custom-stepper" | ||
headerClassName="custom-header" | ||
contentClassName="custom-content" | ||
> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add all of the available properties, I think you are missing the separator, activeStepClass should be activeStepClassName and the orientation property
headerClassName="custom-header" | ||
contentClassName="custom-content" | ||
> | ||
<Step key="1"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this key needed? I can't see it mentioned anywhere in the documentation and in the Step component you are just adding it to the container div. It looks like you are manually creating a key when you add the child components as well.
]; | ||
|
||
return ( | ||
<div style={{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the actual stepper component, otherwise this example is not useful for any future users
No description provided.