-
Notifications
You must be signed in to change notification settings - Fork 11
Extending danger button styling #1280
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
Conversation
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.
Pull request overview
This PR adds styling support for tertiary danger buttons, extending the existing button variant system with a new combination of tertiary styling and danger semantics.
Key Changes
- Added CSS custom properties for tertiary danger buttons in both light and dark themes
- Implemented tertiary danger button styles with hover and active states
- Updated CHANGELOG to document the new feature
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/assets/stylesheets/index.scss | Adds CSS custom properties for tertiary danger button colors in both dark and light themes, defining text color and background colors for hover/active states |
| src/assets/stylesheets/Button.scss | Implements the tertiary danger button variant nested within the danger button modifier, applying the new CSS variables with hover and active state handling |
| CHANGELOG.md | Documents the addition of tertiary danger button styles under the Unreleased section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| &:hover { | ||
| background-color: var(--rpf-button-tertiary-danger-background-color-hover); | ||
| } | ||
|
|
||
| &:active { | ||
| background-color: var(--rpf-button-tertiary-danger-background-color-active); | ||
| } |
Copilot
AI
Dec 5, 2025
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.
The tertiary danger button implementation is missing several important interactive states that are present in other button variants:
-
Missing
:focus-visiblestate: Other button variants (primary, secondary, tertiary, and danger) all have:focus-visiblestyles. The tertiary danger button should also handle focus visibility for accessibility. -
Missing
:disabledstate: The base tertiary button (lines 156-168) includes a:disabledstate with specific styling. The tertiary danger button should also handle the disabled state. -
Missing
.btn-outerwrapper selectors: Other button states use both direct pseudo-classes (e.g.,&:hover) and wrapper-based selectors (e.g.,.btn-outer:hover &) for hover, active, and focus-visible states. The tertiary danger button should follow this same pattern for consistency.
Consider adding these missing states to ensure consistent behavior with other button variants.
| &:hover { | |
| background-color: var(--rpf-button-tertiary-danger-background-color-hover); | |
| } | |
| &:active { | |
| background-color: var(--rpf-button-tertiary-danger-background-color-active); | |
| } | |
| // Hover states | |
| &:hover, | |
| .btn-outer:hover & { | |
| background-color: var(--rpf-button-tertiary-danger-background-color-hover); | |
| } | |
| // Active states | |
| &:active, | |
| .btn-outer:active & { | |
| background-color: var(--rpf-button-tertiary-danger-background-color-active); | |
| } | |
| // Focus-visible states | |
| &:focus-visible, | |
| .btn-outer:focus-visible & { | |
| background-clip: padding-box; | |
| border: 2px solid transparent; | |
| outline: 3px solid $rpf-brand-raspberry; | |
| } | |
| // Disabled states | |
| &:disabled { | |
| background-color: inherit; | |
| color: var(--rpf-button-tertiary-danger-text-color); | |
| cursor: default; | |
| svg { | |
| fill: var(--rpf-button-tertiary-danger-text-color); | |
| } | |
| &:hover, | |
| .btn-outer:hover & { | |
| background-color: inherit; | |
| } | |
| } |
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.
We're using the default focus state for this. We have no use case currently for disabling this and we're not using btn-outer here, so there's no need to add these, and they can be included at a future date if the need arises.
What's Changed?
related to https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/948