Skip to content

Conversation

@loiswells97
Copy link
Contributor

@loiswells97 loiswells97 commented Dec 5, 2025

What's Changed?

  • Added tertiary danger button styles

Danger danger

related to https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/948

@loiswells97 loiswells97 temporarily deployed to previews/1280/merge December 5, 2025 11:06 — with GitHub Actions Inactive
@loiswells97 loiswells97 temporarily deployed to previews/1280/merge December 5, 2025 13:57 — with GitHub Actions Inactive
@loiswells97 loiswells97 temporarily deployed to previews/1280/merge December 5, 2025 14:01 — with GitHub Actions Inactive
Copy link
Contributor

Copilot AI left a 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.

Comment on lines +239 to +245
&:hover {
background-color: var(--rpf-button-tertiary-danger-background-color-hover);
}

&:active {
background-color: var(--rpf-button-tertiary-danger-background-color-active);
}
Copy link

Copilot AI Dec 5, 2025

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:

  1. Missing :focus-visible state: Other button variants (primary, secondary, tertiary, and danger) all have :focus-visible styles. The tertiary danger button should also handle focus visibility for accessibility.

  2. Missing :disabled state: The base tertiary button (lines 156-168) includes a :disabled state with specific styling. The tertiary danger button should also handle the disabled state.

  3. Missing .btn-outer wrapper 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.

Suggested change
&: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;
}
}

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

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.

@loiswells97 loiswells97 merged commit 38e75e0 into main Dec 5, 2025
14 checks passed
@loiswells97 loiswells97 deleted the lois/948-danger-button-styling branch December 5, 2025 15:09
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.

3 participants