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

Add color palette #48 #49

Merged
merged 11 commits into from
Mar 8, 2016
Merged

Add color palette #48 #49

merged 11 commits into from
Mar 8, 2016

Conversation

danhahn
Copy link
Contributor

@danhahn danhahn commented Mar 1, 2016

Standardizing colors

@color-gray-10: fade(#000, 10%);
@color-gray-25: fade(#000, 25%);
@color-gray-30: fade(#000, 30%);
@color-borderColor: @color-gray;
Copy link
Contributor

Choose a reason for hiding this comment

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

when would you use @color-borderColor verses @color-default-borderColor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

default is related to the default button coloring. I would think that we would use @color-borderColor anytime we need to define a border we would set the color using this variable

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I see the distinction either.

@color-white: #fff;
@color-black: black;

@color-gray-5: fade(@color-black, 5%);
Copy link
Contributor

Choose a reason for hiding this comment

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

I vote that we use only one set of grays. It's weird to be using two sets.

@jondlm
Copy link
Contributor

jondlm commented Mar 3, 2016

I think it makes more sense to move nearly all the logic in the example in to the ColorPalette component since it's really specific to our library.

cc @ogupte @nathaniellee

@@ -8,11 +8,7 @@
display: inline-block;
outline: none;
border: 1px solid @color-default-borderColor;
border-radius: @size-button-borderRadius;
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to remove button styles?

@@ -77,3 +78,222 @@ html {
}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

should these styles perhaps live in their own less file at src/docs/containers/colors.less maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably, I've just been throwing everything in index until it grows too big.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed for this PR I think though

danhahn added a commit that referenced this pull request Mar 8, 2016
@danhahn danhahn merged commit 684f662 into master Mar 8, 2016
@danhahn danhahn deleted the feature/ANXR-171 branch March 8, 2016 16:52
@jondlm jondlm mentioned this pull request Mar 8, 2016
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.

5 participants