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

Clean up button files #654

Merged
merged 3 commits into from
Feb 22, 2017
Merged

Clean up button files #654

merged 3 commits into from
Feb 22, 2017

Conversation

llorca
Copy link
Contributor

@llorca llorca commented Feb 9, 2017

Changes proposed in this pull request:

  • Remove extend usage
  • Move button spinner rules (not needed inside generic mixins, correct me if I'm wrong)

@blueprint-bot
Copy link

Remove @extend usage

Preview: docs
Coverage: core | datetime

@@ -65,6 +66,10 @@ Styleguide components.button.css
@each $intent, $colors in $button-intents {
&.pt-intent-#{$intent} {
@include pt-button-intent($colors...);

.pt-button-spinner .pt-spinner-head {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this inside the @each loop but the dark block on line 178 below isn't?

much preferable to define this static rule once instead of redefining for each intent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So true

Copy link
Contributor

@giladgray giladgray left a comment

Choose a reason for hiding this comment

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

(blocking merge for question above)

@blueprint-bot
Copy link

Move loading outside intent loop

Preview: docs
Coverage: core | datetime

@cmslewis
Copy link
Contributor

@giladgray is this good to merge now? Looks like @llorca addressed the question above.

# Conflicts:
#	packages/core/src/components/button/_common.scss
@blueprint-bot
Copy link

Merge remote-tracking branch 'origin/master' into al/btn-refactor

Preview: docs
Coverage: core | datetime

@giladgray giladgray merged commit 4089829 into master Feb 22, 2017
@giladgray giladgray deleted the al/btn-refactor branch February 22, 2017 19:24
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.

None yet

4 participants