-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 :disabled support for pt-file-upload #717
Conversation
@@ -84,6 +84,14 @@ $control-group-stack: ( | |||
} | |||
} | |||
|
|||
@mixin pt-input-disabled() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixins don't need to come before they're declared. Similar to what we do for buttons, I prefer having pt-input()
first followed by its modifiers pt-input-<modifier>()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got a small nit, otherwise looks pretty good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question about mixin API but this looks really good overall so I'm gonna approve it now.
} | ||
|
||
.pt-button-spinner .pt-spinner-head { | ||
stroke: $dark-progress-head-color; | ||
} | ||
} | ||
|
||
@mixin pt-dark-button-intent-disabled() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not crazy about adding four new mixins to this file, but I get it. I wonder if it's reasonable to just add some params to the first disabled mixin for color
and background
?
@cmslewis merge conflicts ahead! |
Verified that buttons, text inputs, and file upload all look good on Chrome/FF/Safari/IE. Merging. |
Fixes #689
Checklist
Changes proposed in this pull request:
:disabled
onpt-file-upload
pt-button
/pt-input
disabled styles into reusable mixins:disabled
state.Reviewers should focus on:
Screenshot
Also, buttons are visually unchanged:
And inputs are visually unchanged: