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

Added !default to core colors #377

Merged
merged 1 commit into from
Jan 6, 2017
Merged

Conversation

gregtillbrook
Copy link
Contributor

Small PR that adds !default to the core blueprint colors. I know this was discussed already internally by you folks but following discussion in #123 I thought Id create this PR incase you agreed with the line of reasoning there. If not, then please close/delete this PR :)

Sass lint is passing.

@palantirtech
Copy link
Member

Thanks for your interest in palantir/blueprint, @gregtillbrook! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@llorca
Copy link
Contributor

llorca commented Dec 16, 2016

Thanks @gregtillbrook! Let's keep this open, other team members will review in a couple weeks after the vacation 👍

@patresk
Copy link

patresk commented Dec 20, 2016

+1! This would be great.

@giladgray
Copy link
Contributor

giladgray commented Jan 3, 2017

some relevant conversation begins here: #396 (comment)

i'm inclined to just ship this because it'll make consumption much easier for everyone. modifying the named colors will be frowned upon but i recognize that it's much simpler to just modify $blue3 instead of the many aliases that map to it.

@adidahiya
Copy link
Contributor

@gregtillbrook Circle is not reporting build status -- would you mind pushing an empty commit to try and trigger CI?

@giladgray
Copy link
Contributor

giladgray commented Jan 5, 2017

@gregtillbrook if we don't hear from you by end of today, I'll take over this feature so we can ship it in tomorrow's release 👍

@Santas
Copy link

Santas commented Jan 6, 2017

Is it possible to add !default to $button-intents as well https://github.com/gregtillbrook/blueprint/blob/12835c96927a87289a76a12d1763dd2b81569a57/packages/core/src/components/button/_common.scss#L54 or should that go to a separate PR?

@isTravis
Copy link

isTravis commented Jan 6, 2017

@Santas, that also came up in the discussion on #396 and seems to have just been a missed line. @giladgray fixed it already in this commit, so I'd assume button-intents will have !default in the next release.

@giladgray giladgray merged commit 4d4356e into palantir:master Jan 6, 2017
@gregtillbrook
Copy link
Contributor Author

Sorry I didn't reply - was on a long haul flight. Thanks for accepting this one! :)

@giladgray giladgray mentioned this pull request Jan 6, 2017
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.

8 participants