-
Notifications
You must be signed in to change notification settings - Fork 2.8k
console: migrate WarningSymbol and TextInput to TypeScript #4314 #4562
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
console: migrate WarningSymbol and TextInput to TypeScript #4314 #4562
Conversation
|
Beep boop! 🤖 Hey @fkhadra, thanks for your PR! One of my human friends will review this PR and get back to you as soon as possible. Stay awesome! 😎 |
|
Review app for commit 3ffd81c deployed to Heroku: https://hge-ci-pull-4562.herokuapp.com |
beerose
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.
Perfect 🎉
Left one comment.
| const WarningSymbol = ({ | ||
| export interface WarningSymbolProps { | ||
| tooltipText: string; | ||
| tooltipPlacement?: string; |
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.
We can narrow it to 'left' | 'right' | 'top' | 'bottom'.
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.
Fixed. Thanks for the review 🙏
|
Review app for commit aaf50da deployed to Heroku: https://hge-ci-pull-4562.herokuapp.com |
|
Review app for commit 7143546 deployed to Heroku: https://hge-ci-pull-4562.herokuapp.com |
|
Review app for commit 55540e8 deployed to Heroku: https://hge-ci-pull-4562.herokuapp.com |
|
Beep boop! 🤖 Awesome work @fkhadra! All of us at Hasura ❤️ what you did. Thanks again 🤗 |
|
Review app https://hge-ci-pull-4562.herokuapp.com is deleted |
Description
Hey,
This PR migrates
WarningSymbolandTextInputto TypeScript. For the typing ofreact-bootstrap/lib/OverlayTrigger, I simply declared the module in thedeclaration.d.tsfile, the same as it's done forreact-bootstrap/lib/OverlayTooltip.I did not copy any types from DefinitelyTyped because this PR will bump the version of
react-bootstrapand add the typing as well.Changelog
CHANGELOG.mdis updated with user-facing content relevant to this PR.Affected components
Related Issues
Related issue #4314.
Catalog upgrade
Does this PR change Hasura Catalog version?
Metadata
Does this PR add a new Metadata feature?
GraphQL
Breaking changes