Skip to content
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

Closed
wants to merge 2 commits into from
Closed

[WIP] CSS resets #94

wants to merge 2 commits into from

Conversation

breadadams
Copy link
Member

Close #92

@@ -7,7 +7,7 @@ import { Content } from './CardContent'
import { media } from '../../utils'

const EmptyImage = CardImage.extend`
${emptyStateImageAnimation}
${emptyStateImageAnimation};
Copy link
Member

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;
Copy link
Member

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;
Copy link
Member

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`
Copy link
Member

@Kikobeats Kikobeats Mar 25, 2018

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};

Copy link
Member

@Kikobeats Kikobeats left a 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;
* {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh, sure?

@Kikobeats
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add missing CSS resets for links
2 participants