Add new Button component - #3266
Conversation
|
Pull request environment is available at https://stoctodocspr3266.z22.web.core.windows.net. You can view the ephemeral environment status in Octopus Deploy. This environment will be automatically deprovisioned when the pull request is closed, or after 7 days of inactivity. |
ed53c59 to
a634ace
Compare
| }, | ||
| "devDependencies": { | ||
| "@playwright/test": "^1.59.1", | ||
| "@playwright/test": "^1.60.0", |
There was a problem hiding this comment.
I ran into a hanging installation issue, bumped to get the fix: microsoft/playwright#41133
a634ace to
e3c76c4
Compare
e3c76c4 to
11d95f5
Compare
| text-align: center; | ||
| } | ||
|
|
||
| .btn--medium { |
There was a problem hiding this comment.
Class naming to fit the BEM convention, which seems to be what's followed elsewhere
ellen-octopus
left a comment
ellen-octopus
left a comment
There was a problem hiding this comment.
Looks great, thanks @rosslovas 👏🏻
|
|
||
| /* Button states */ | ||
| .btn:hover:not(:disabled) { | ||
| background: var(--colorBackgroundPrimaryHover); |
There was a problem hiding this comment.
Nitpick: I presume the mix of tokens between colorBackgroundPrimaryHover and colorButtonBackgroundDisabled is intentional? Ideally it would all be a semantic token with Button or a generic token
There was a problem hiding this comment.
Yeah, intentional on my part at least by virtue of following the design as-is in Figma. It's a good point, I'll note it down as something to discuss with the team when I get a chance, thank you!
4 participants
Summary
Adds a new
<Button label="Label" />component.Results
Target design:
Screenshots
As implemented:
See in action at https://stoctodocspr3266.z22.web.core.windows.net/components#button
Light mode
Dark mode
(note the background colour doesn't match the target design (yet), but the buttons themselves should)
