Skip to content

Conversation

@jouni
Copy link
Member

@jouni jouni commented Nov 21, 2025

  • Refactor [theme~=red|orange|yellow|green|blue|purple] accent-color modifiers to .aura-accent-red|orange|yellow|green|blue|purple.
  • Add neutral accent color modifier (move it from aura-badge).
  • Refactor [theme=badge] to .aura-badge, including .aura-badge-filled and .aura-badge-point.
  • Combine accent-color overrides into color.css, which were duplicated in component-specific stylesheets.
  • Add a test/dev page for Aura colors, which allows testing multiple combinations easily.

@jouni jouni requested review from Copilot and web-padawan November 21, 2025 14:09
Copilot finished reviewing on behalf of jouni November 21, 2025 14:14
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 refactors the Aura theme's accent color system by migrating from theme attribute-based modifiers ([theme~="red"]) to CSS class-based modifiers (.aura-accent-red). The refactoring consolidates duplicated color override CSS rules from component-specific stylesheets into a centralized color.css file, improving maintainability. A new neutral accent color modifier is introduced, and badge styling is migrated from [theme~="badge"] to .aura-badge classes. Additionally, a comprehensive color testing page is added to facilitate development and testing.

Key changes:

  • Introduced CSS classes for accent color modifiers (.aura-accent-[color]) replacing theme attributes
  • Consolidated color overrides from button.css, tabs.css, and side-nav.css into color.css
  • Refactored badge styling to use CSS classes (.aura-badge, .aura-badge-filled, .aura-badge-point)

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/aura/src/color.css Added accent color CSS class selectors, refactored accent surface calculation algorithm, consolidated "restore accent color" and "contrast color" rules from component stylesheets
packages/aura/src/components/button.css Removed duplicated color override rules and "restore accent color" selector (consolidated into color.css)
packages/aura/src/components/tabs.css Removed duplicated color override rules, updated filled variant selector to use child combinator
packages/aura/src/components/side-nav.css Removed duplicated color override rules, updated filled variant selector to use child combinator
dev/aura/badge.css Migrated from theme attributes to CSS classes, added special handling for filled badges within filled variants
dev/aura/colors.html New test/dev page for testing Aura color combinations with interactive controls
dev/aura.html Updated all badge instances from theme attributes to CSS classes throughout the demo page

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const grid = view.querySelector('vaadin-grid');
grid.items = generateItems(20);
grid.selectedItems = [grid.items[0]];
const badgeColors = ['accent', 'green', 'yellow', 'purple', 'orange', 'blue', 'red'];
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The badgeColors array uses 'accent' which will generate the invalid class aura-accent-accent. It should be 'color' to generate the correct class aura-accent-color:

const badgeColors = ['color', 'green', 'yellow', 'purple', 'orange', 'blue', 'red'];
Suggested change
const badgeColors = ['accent', 'green', 'yellow', 'purple', 'orange', 'blue', 'red'];
const badgeColors = ['color', 'green', 'yellow', 'purple', 'orange', 'blue', 'red'];

Copilot uses AI. Check for mistakes.
Comment on lines +182 to 210
:is(#id, .aura-accent-green) {
--aura-accent-color-light: var(--aura-green);
--aura-accent-color-dark: var(--aura-green);
--aura-accent-color: light-dark(var(--aura-accent-color-light), var(--aura-accent-color-dark));
}

:is(#id, [theme~='yellow'], [theme~='warning']) {
:is(#id, .aura-accent-yellow) {
--aura-accent-color-light: var(--aura-yellow);
--aura-accent-color-dark: var(--aura-yellow);
--aura-accent-color: light-dark(var(--aura-accent-color-light), var(--aura-accent-color-dark));
}

:is(#id, [theme~='orange']) {
:is(#id, .aura-accent-orange) {
--aura-accent-color-light: var(--aura-orange);
--aura-accent-color-dark: var(--aura-orange);
--aura-accent-color: light-dark(var(--aura-accent-color-light), var(--aura-accent-color-dark));
}

:is(#id, [theme~='purple']) {
:is(#id, .aura-accent-purple) {
--aura-accent-color-light: var(--aura-purple);
--aura-accent-color-dark: var(--aura-purple);
--aura-accent-color: light-dark(var(--aura-accent-color-light), var(--aura-accent-color-dark));
}

:is(#id, [theme~='blue'], [theme~='info']) {
:is(#id, .aura-accent-blue) {
--aura-accent-color-light: var(--aura-blue);
--aura-accent-color-dark: var(--aura-blue);
--aura-accent-color: light-dark(var(--aura-accent-color-light), var(--aura-accent-color-dark));
}
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The theme attribute selectors [theme~='success'], [theme~='warning'], and [theme~='info'] were removed during this refactor, while [theme~='danger'] and [theme~='error'] were preserved for red. This creates an inconsistency in backwards compatibility. Consider adding them back for consistency:

:is(#id, [theme~='success'], .aura-accent-green) {
  --aura-accent-color-light: var(--aura-green);
  --aura-accent-color-dark: var(--aura-green);
  --aura-accent-color: light-dark(var(--aura-accent-color-light), var(--aura-accent-color-dark));
}

:is(#id, [theme~='warning'], .aura-accent-yellow) {
  --aura-accent-color-light: var(--aura-yellow);
  --aura-accent-color-dark: var(--aura-yellow);
  --aura-accent-color: light-dark(var(--aura-accent-color-light), var(--aura-accent-color-dark));
}

:is(#id, [theme~='info'], .aura-accent-blue) {
  --aura-accent-color-light: var(--aura-blue);
  --aura-accent-color-dark: var(--aura-blue);
  --aura-accent-color: light-dark(var(--aura-accent-color-light), var(--aura-accent-color-dark));
}

Copilot uses AI. Check for mistakes.
Comment on lines +29 to +36
> .aura-badge-filled:not(
.aura-accent-red,
.aura-accent-orange,
.aura-accent-yellow,
.aura-accent-green,
.aura-accent-blue,
.aura-accent-purple
) {
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .aura-accent-neutral class is missing from the :not() selector. When a neutral-colored filled badge is placed within a filled variant (like a primary button), it should maintain its explicit color like the other color variants do. Consider adding .aura-accent-neutral to the exclusion list:

> .aura-badge-filled:not(
    .aura-accent-neutral,
    .aura-accent-red,
    .aura-accent-orange,
    .aura-accent-yellow,
    .aura-accent-green,
    .aura-accent-blue,
    .aura-accent-purple
  )

Copilot uses AI. Check for mistakes.
Comment on lines 103 to 121
.level--1 {
--aura-surface-level: -1;
}

.level-0 {
--aura-surface-level: 0;
}

.level-2 {
--aura-surface-level: 2;
}

.level-3 {
--aura-surface-level: 3;
}

.level-4 {
--aura-surface-level: 4;
}
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing CSS class definition for .level-1. The class is used in the HTML at lines 142 and 150, but there's no corresponding CSS rule to set --aura-surface-level: 1. Add:

.level-1 {
  --aura-surface-level: 1;
}

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be needed as --aura-surface-level: 1 is the default one.

}

/* Restore accent color for child elements like badges */
:where(vaadin-button, vaadin-menu-bar-button, vaadin-drawer-toggle) > *,
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vaadin-crud-edit is missing from this selector. It was included in the original rule in button.css (:where(vaadin-button, vaadin-menu-bar-button, vaadin-drawer-toggle, vaadin-crud-edit) > *) and should be consolidated here as well. Add it to maintain consistency:

:where(vaadin-button, vaadin-menu-bar-button, vaadin-drawer-toggle, vaadin-crud-edit) > *,
Suggested change
:where(vaadin-button, vaadin-menu-bar-button, vaadin-drawer-toggle) > *,
:where(vaadin-button, vaadin-menu-bar-button, vaadin-drawer-toggle, vaadin-crud-edit) > *,

Copilot uses AI. Check for mistakes.
}

/* Badges and other content that uses contrast color inside filled variants */
:is(vaadin-button, vaadin-menu-bar-button, vaadin-drawer-toggle)[theme~='primary'] > *,
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vaadin-crud-edit is missing from this selector. It should be included alongside the other button components since it can have the theme~='primary' attribute (as seen in button.css line 52). Update to:

:is(vaadin-button, vaadin-menu-bar-button, vaadin-drawer-toggle, vaadin-crud-edit)[theme~='primary'] > *,
Suggested change
:is(vaadin-button, vaadin-menu-bar-button, vaadin-drawer-toggle)[theme~='primary'] > *,
:is(vaadin-button, vaadin-menu-bar-button, vaadin-drawer-toggle, vaadin-crud-edit)[theme~='primary'] > *,

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

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