feat(ProgressStepper): updating description prop to ReactNode #8017
Conversation
|
Please let me know if it is okay to leave all of the examples that use |
|
Preview: https://patternfly-react-pr-8017.surge.sh A11y report: https://patternfly-react-pr-8017-a11y.surge.sh |
There was a problem hiding this comment.
This looks good! I think it's fine to leave the other examples as-is with a string passed in and to update the example like you did. I only had one nitpick below. It could also be worth updating the "with alignment" example similarly, but not really a blocker on that.
EDIT: based on Titani's response below, other than her requested change it looks good!
...s/react-core/src/components/ProgressStepper/examples/ProgressStepperBasicWithDescription.tsx
Outdated
Show resolved
Hide resolved
| @@ -15,7 +15,13 @@ export const ProgressStepperBasicWithDescription: React.FunctionComponent = () = | |||
| <ProgressStep | |||
| variant="info" | |||
| isCurrent | |||
| description="This is the second thing to happen" | |||
| description={ | |||
There was a problem hiding this comment.
Three is no need to update the example. If you want to add a test to verify the change that is fine. I do not think that is necessary though.
There was a problem hiding this comment.
Sounds good! I'll revert the example back and add a test. Thanks @thatblindgeye and @tlabaj !!
There was a problem hiding this comment.
@tlabaj it appears that there's no way to test <br /> through react testing according to testing-library/dom-testing-library#750
There was a problem hiding this comment.
That is ok. It is not really a necessary test.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #7801 . Updated the
descriptionprop inProgressStep.tsxto allow for the use of line breaks. This was demonstrated in the updated example ofProgressStepperWithBasicDescription.tsxin the second step.Additional issues: