Skip to content
This repository was archived by the owner on Mar 19, 2021. It is now read-only.

Conversation

mg901
Copy link
Contributor

@mg901 mg901 commented Jun 27, 2019

No description provided.


export const H1 = styled.h1`
margin: ${(p) => (p.narrow ? 0 : 3)}rem 0;
/* margin: ${(p) => (p.narrow ? 0 : 3)}rem 0; */
Copy link

@artalar artalar Jun 27, 2019

Choose a reason for hiding this comment

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

Why commented out?

import { GlobalStyles } from "./global-styles"

export const config = {
base: "14px",
Copy link

Choose a reason for hiding this comment

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

Why is not rems?

Copy link
Member

Choose a reason for hiding this comment

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

Typographist requires base in pixels. Because pixels is absolute independent unit

</GridCard>
</CardBox>
</CardNarrow>
<Rhythm>
Copy link
Member

Choose a reason for hiding this comment

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

Forget to remove rhythm?

import PropTypes from "prop-types"
import styled from "styled-components"
import { above } from "styled-breakpoints"
import { up } from "@typographist/styled"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe remove styled-breakpoints from dependencies?

user-select: none;
`

H4.propTypes = {
Copy link

@stuneak stuneak Jun 28, 2019

Choose a reason for hiding this comment

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

Wouldn't it be better to use flow instead of proptypes?

@sergeysova sergeysova merged commit 73c54fd into dev Jul 1, 2019
@sergeysova sergeysova deleted the feat/cards-typography branch July 1, 2019 09:16
@sergeysova
Copy link
Member

@all-contributors please add @mg901 for design, code

@allcontributors
Copy link
Contributor

@sergeysova

I've put up a pull request to add @mg901! 🎉

@sergeysova
Copy link
Member

@all-contributors please add @artalar for review

@allcontributors
Copy link
Contributor

@sergeysova

I've put up a pull request to add @artalar! 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants