-
Notifications
You must be signed in to change notification settings - Fork 377
refactor(Hint): Convert hint examples to typescript #7487
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-7487.surge.sh A11y report: https://patternfly-react-pr-7487-a11y.surge.sh |
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.
Only had a couple of nitpicks below, but otherwise it's looking good!
| KebabToggle | ||
| } from '@patternfly/react-core'; | ||
|
|
||
| export const BasicHint: React.FunctionComponent = () => { |
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.
Nit: for the most part the exported component name within files usually matches the file name, so this would be export const HintBasicWithTitle....
| KebabToggle | ||
| } from '@patternfly/react-core'; | ||
|
|
||
| export const BasicHint: React.FunctionComponent = () => { |
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.
Similar nitpick to the previous comment re: the exported component name matching the file name
wise-king-sullyman
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 great! 🚀
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
What: Closes #7482