-
Notifications
You must be signed in to change notification settings - Fork 377
chore(InputGroup): convert examples to TypeScript #7493
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
Conversation
|
Preview: https://patternfly-react-pr-7493.surge.sh A11y report: https://patternfly-react-pr-7493-a11y.surge.sh |
3bbf002 to
52e5348
Compare
thatblindgeye
left a comment
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.
Things are looking good! Just had a couple of suggested changes and one comment that I tagged Titani for input on.
packages/react-core/src/components/InputGroup/examples/InputGroupBasic.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/components/InputGroup/examples/InputGroupWithDropdown.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/components/InputGroup/examples/InputGroupWithDropdown.tsx
Outdated
Show resolved
Hide resolved
52e5348 to
1bc01a0
Compare
| setIsOpen(isOpen); | ||
| }; | ||
|
|
||
| const onSelect = event => { |
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.
If you keep this function after addressing eric's comment, could you add a type to the params of functions you pass to components? This helps us sanity check that our component props are typed correctly and also helps communicate types easier to people using the component.
packages/react-core/src/components/InputGroup/examples/InputGroupWithDropdown.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Tomas Psota <tpsota@redhat.com>
1bc01a0 to
52b0ce9
Compare
thatblindgeye
left a comment
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.
🎉 looks good! Great work on this
tlabaj
left a comment
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.
LGTM! Great work.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
Signed-off-by: Tomas Psota tpsota@redhat.com
What: Closes #7490