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

OIO Defaults #38

Merged
merged 5 commits into from
Jan 31, 2017
Merged

OIO Defaults #38

merged 5 commits into from
Jan 31, 2017

Conversation

stevenyuen
Copy link
Contributor

No description provided.

import style from './style.less'

// TODO: Deprecate fontFamily and primaryColor props
Copy link
Member

Choose a reason for hiding this comment

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

@stevenyuen Any reason not to deprecate primaryColor and fontFamily in this PR? (At least internally)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@asabhaney just would prefer to do that in a separate PR since lots of components use that primaryColor prop.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. I'll this as an issue.

import style from './style.less'

// TODO: Deprecate fontFamily and primaryColor props
Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. I'll this as an issue.

}
},
viewport: {
minSizes: {
Copy link
Member

Choose a reason for hiding this comment

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

Was just thinking, how do you feel about using the word breakpoints? Just an idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@asabhaney good idea.

Copy link
Contributor

@jaredreich jaredreich left a comment

Choose a reason for hiding this comment

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

Nice! Looks good to me.

b: '768px',
c: '992px',
d: '1300px',
e: '1650px'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this is using characters instead of numbers because of our breakpoint system that uses the 2[b-e] style syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly. Right on sir.

@@ -12,7 +14,8 @@ export default class OIO extends Component {

static defaultProps = {
fontFamily: 'Helvetica Neue, Arial',
primaryColor: '#78909C'
primaryColor: '#78909C',
...defaults
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@asabhaney asabhaney merged commit 52a8225 into master Jan 31, 2017
@asabhaney asabhaney deleted the constants branch January 31, 2017 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants