feat(Wizard - next): supporting component unit tests #7731
Conversation
|
Preview: https://patternfly-react-pr-7731.surge.sh A11y report: https://patternfly-react-pr-7731-a11y.surge.sh |
jpuzz0
force-pushed
the
feature/composable-wizard-more-tests
branch
from
057f828 to
8eb62e9
Compare
jpuzz0
changed the title
jpuzz0
force-pushed
the
feature/composable-wizard-more-tests
branch
from
8eb62e9 to
8f07d76
Compare
jpuzz0
changed the title
jpuzz0
force-pushed
the
feature/composable-wizard-more-tests
branch
from
8f07d76 to
3eb5f6c
Compare
| goToStepByIndex: (index: number) => void; | ||
| /** The button's aria-label */ | ||
| /** Custom WizardNav or callback used to create a default WizardNav */ | ||
| nav?: DefaultWizardNavProps | CustomWizardNavFunction; |
There was a problem hiding this comment.
Would it be possible/wise to have the nav and the footer both use the same pattern? Right now, the custom footer is a props object or a react node and the nav is a props object or a function.
There was a problem hiding this comment.
I don't think it's necessary, but we could make a CustomWizardFooterFunction if you want that returns onNext, onBack, and onClose. Thoughts on that change?
There was a problem hiding this comment.
That'd be helpful if people want a custom footer with custom buttons, right? so maybe that'd be wise
There was a problem hiding this comment.
I agree. That will make things complicated with how the footer is managed in context. We'd end up initializing footer's state variable as a function, which I'm not sure is even possible. Maybe I can try to do this in the next PR here where I'm updating a bunch of types (and including a new one called CustomWizardNavItemFunction - very similar to CustomWizardNavFunction):
https://github.com/patternfly/patternfly-react/pull/7915/files#diff-087f56845137e426d26fe8b189b155902a59de75ee7e2beff5c07db4d4f9c25f
Unless you think this is a blocking change for this PR.
There was a problem hiding this comment.
I don't think it's blocking since all this is beta :)
There was a problem hiding this comment.
Ok cool. I will address it in that other PR then, thanks!
wise-king-sullyman
left a comment
wise-king-sullyman
left a comment
There was a problem hiding this comment.
Unfortunately I won't have time to review this in depth until I return from PTO on Friday 9/9, but one thing that I know will need to change is that the userEvent usage will need to be updated to be compatible with user-event v14+
jpuzz0
force-pushed
the
feature/composable-wizard-more-tests
branch
from
3eb5f6c to
0007e01
Compare
This has been addressed. |
wise-king-sullyman
left a comment
wise-king-sullyman
left a comment
There was a problem hiding this comment.
I've got a couple of nits and a couple of questions about places where I think it would be better to test behavior on a sub-component directly.
packages/react-core/src/next/components/Wizard/__tests__/Wizard.test.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/next/components/Wizard/__tests__/WizardBody.test.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/next/components/Wizard/__tests__/WizardToggle.test.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/next/components/Wizard/__tests__/WizardToggle.test.tsx
Outdated
Show resolved
Hide resolved
jpuzz0
force-pushed
the
feature/composable-wizard-more-tests
branch
from
20a54bd to
b36baeb
Compare
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
5 participants
What: Closes #7737