console: migrate AlertBox component to typescript #4721
Conversation
|
Beep boop! 🤖 Hey @harishm72, 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 4f01575 deployed to Heroku: https://hge-ci-pull-4721.herokuapp.com |
|
Review app for commit 07b7f7c deployed to Heroku: https://hge-ci-pull-4721.herokuapp.com |
beerose
left a comment
There was a problem hiding this comment.
Thanks for your PR! 🎉
Right now AlertBox only accepts children, type, pointer and size.
However, we should also be able to pass all the style props that are accepted by StyledAlertBox and HTML div element props.
We could declare StyledAlertBoxProps as:
interface StyledAlertBoxOwnProps
extends FlexboxProps,
ColorProps,
BorderProps,
TypographyProps,
LayoutProps,
SpaceProps,
ShadowProps,
React.ComponentProps<'div'> {}But styled-components and TypeScript doesn't work well together, and above interface would cause a type error (the type of prop color from styled-system conflicts with div element's color.). So we need to deal with it slightly differently.
Here is how to do this:
- Crate component
Boxand interfaceBoxPropsinAlertBox.ts. Another PR adds it to the separate folder, but as it's not yet merged, we can keep itAlertBox.tsfile.
interface BoxProps extends Omit<React.ComponentPropsWithRef<'div'>, 'color'> {}
const Box = ('div' as any) as React.FC<BoxProps>;- Create an interface
StyledAlertBoxOwnProps:
interface StyledAlertBoxOwnProps
extends FlexboxProps,
ColorProps,
BorderProps,
TypographyProps,
LayoutProps,
SpaceProps,
ShadowProps {}- Declare
StyledAlertBoxusingBoxand newly created interfaceStyledAlertBoxProps:
export const StyledAlertBox = styled(Box)<StyledAlertBoxProps>`
${flexbox};
${color}
${border}
${typography}
${layout}
${space}
${shadow}
/* Alert type text */
span {
text-transform: capitalize;
}
`;- Export
StyledSpinnerPropsas so that we include both style props and HTML div element attributes:
export interface StyledAlertBoxProps extends StyledAlertBoxOwnProps, BoxProps {}- Then in
index.jsdeclare props type as:
export interface AlertBoxProps
extends IconProps,
Omit<StyledAlertBoxProps, 'size'> {
type: keyof Theme['alertBox'];
}In the above type I replaced type so that it depends on the Theme interface. I also removed children as it's already included because you're using React.FC. I also needed to omit size as it conflicts with the IconProps.
|
Deploy preview for hasura-docs ready! Built with commit b1e0205 |
|
Review app for commit 29a5b8a deployed to Heroku: https://hge-ci-pull-4721.herokuapp.com |
|
Review app for commit b1e0205 deployed to Heroku: https://hge-ci-pull-4721.herokuapp.com |
|
Review app for commit 9fbbec5 deployed to Heroku: https://hge-ci-pull-4721.herokuapp.com |
|
Review app for commit c04241d deployed to Heroku: https://hge-ci-pull-4721.herokuapp.com |
|
Review app for commit 0a02896 deployed to Heroku: https://hge-ci-pull-4721.herokuapp.com |
|
Review app https://hge-ci-pull-4721.herokuapp.com is deleted |
|
Beep boop! 🤖 Awesome work @harishm72! All of us at Hasura ❤️ what you did. Thanks again 🤗 |
|
@harishm72 thank you so much for your contribution 🙏 as a thank you, we'd love to send you some swag. If you're interested, please send an email to marion@hasura.io, and we'll take it from there 🙂 |
Affected components
Related Issues
#4314 (comment)
Breaking changes