-
Notifications
You must be signed in to change notification settings - Fork 889
chore(site): remove Typography component #10769
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
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.
thank you!
@@ -40,24 +39,21 @@ export const EmptyState: FC<React.PropsWithChildren<EmptyStateProps>> = ( | |||
}} | |||
{...boxProps} | |||
> | |||
<Typography variant="h5" css={{ fontSize: 24 }}> |
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.
I would expect an <h5>
to at least have a weight of 500
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.
I agree, I think this could be something we should set as a global style? Wondering if as part of the theme thing, we could include the TailwindCSS reset.
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.
but my point was that here, you manually set it to 400. it should be a <p>
or something instead, not a header.
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.
Ahhh ok, but it is a title so I think makes sense to make it a heading element 🤔
css={{ | ||
textAlign: "center", | ||
fontSize: 32, | ||
fontWeight: 400, | ||
margin: 0, | ||
marginTop: 16, | ||
marginBottom: 32, | ||
lineHeight: 1.25, | ||
|
||
& strong { | ||
font-weight: 600; | ||
} | ||
`} | ||
variant="h1" | ||
"& strong": { | ||
fontWeight: 600, | ||
}, | ||
}} |
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.
aww, why get rid of the css
? I kinda like those for when we have nested selectors
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.
Auto complete and formatting basically
Related to #10612