Introduce styles for disabled buttons with primary, secondary and danger variants.
Also added :hover background color change for secondary and danger variants, because it was missing.
Following styles in Desktop App and Mobile Figma design:
Disabled Primary button - https://www.figma.com/file/a1nkbWgbgjRlrOY9LVurTz/Comm-%2F-Desktop-app?node-id=1077%3A69208
Secondary buttons - https://www.figma.com/file/2H0BQsi8MgC5MpjNtViJxB/Comm-%2F-Mobile-Design-System?node-id=480%3A4797
Details
Currently existing buttons in web app should have proper disabled state - e.g. the ones in thread settings.
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Looks good. Had a couple of questions. Additionally, could you add the figma link?
web/components/button.css | ||
---|---|---|
24 ↗ | (On Diff #11774) | why switch all of these to background-color? `background is shorthand, does the same thing? I find it a bit easier to read: background: blue; color: red You know what the background is and what the text color is`. When I see background-color then ask what color is. Additionally, it's a bit more typing. I'm not really married to background but I think we should stick to one way or another for consistency. |
web/components/button.css | ||
---|---|---|
24 ↗ | (On Diff #11774) | I wanted to standardize the convention in this file, since it contained both background and background-color. I did quick searched and found, that in web styles, background is used ~35 times, and background-color over 60 times, so I used the latter. I don't think there is a big difference between these forms, and I'm ok if we decide to stick to any of them. |
web/components/button.css | ||
---|---|---|
24 ↗ | (On Diff #11774) | There's a significant difference between background and background-color. The shorthand sets a lot of props, not only the color. If we have a style where we set e.g. background-image it will be unset by calling background: #color. Overall, both of the props have their usages, but usually background is what we really want: we want to set a background to be a solid color. On the other hand, if we introduce some global background props, like e.g. background-clip (if we ever have to), then using background would effectively unset them. So currently, it looks like using background is better, because it expresses our intention, but we might need to change that in the future. |