-
Notifications
You must be signed in to change notification settings - Fork 42
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
[WIP] CSS resets #94
[WIP] CSS resets #94
Conversation
@@ -7,7 +7,7 @@ import { Content } from './CardContent' | |||
import { media } from '../../utils' | |||
|
|||
const EmptyImage = CardImage.extend` | |||
${emptyStateImageAnimation} | |||
${emptyStateImageAnimation}; |
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.
is the semicolon necessary? 🤔
<NanoClamp className={className} lines={lines} text={children} is='p' /> | ||
) | ||
|
||
const CardText = styled(Clamp)` | ||
&&& { | ||
font-family: inherit; | ||
font-size: inherit; |
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.
are this inherit
styles resets?
border-color: #E1E8ED; | ||
background: #fff; | ||
border: 1px solid #e1e8ed; | ||
margin: 0; |
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 a reset
background-color: inherit; | ||
` | ||
|
||
export const sizeReset = css` |
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 was thinking more in a file that content reset per each component, for example:
export const cardDescription = css`
text-decoration: none;
`
Then when you load the specific reset style into the component is easy to determinate what style are part of the component and what are part of the reset:
// resets
${reset.cardDescription};
// specific styles
font-size: 14px;
flex-grow: 2;
margin: auto 0;
line-height: 18px;
${({cardSize}) => !isLarge(cardSize) && mobileDescriptionStyle};
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 feel we can extract more reset present currently in the code and isolate it following a different approach
&:active, | ||
&:hover { | ||
outline: 0; | ||
* { |
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.
uh, sure?
Do you think we still need that? The main reset styles are related with vanilla bundle and we added a recipe into doc to handle it properly Feeling that this is enough |
86f490e
to
23b5dfb
Compare
Close #92