Skip to content

scale-color $lightness must use $secondary for dark themes#7

Open
everettbu wants to merge 1 commit into
theme-color-scheme-pre-copilotfrom
theme-color-scheme-post-copilot
Open

scale-color $lightness must use $secondary for dark themes#7
everettbu wants to merge 1 commit into
theme-color-scheme-pre-copilotfrom
theme-color-scheme-post-copilot

Conversation

@everettbu

Copy link
Copy Markdown

Test 7

Copilot AI review requested due to automatic review settings July 26, 2025 19:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates SCSS color declarations to use proper theme-aware color functions for dark theme compatibility. The changes replace direct scale-color($primary, $lightness: X%) calls with dark-light-choose() functions that select appropriate color values for both light and dark themes.

  • Replaces single-theme color scaling with theme-aware color selection
  • Ensures proper contrast and readability in dark themes using $secondary color base
  • Maintains consistent lightness adjustments across theme modes

Reviewed Changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
mobile/*.scss Updates mobile-specific styling with dark-theme-aware color functions
desktop/*.scss Updates desktop-specific styling with dark-theme-aware color functions
common/base/*.scss Updates shared base styling components with dark-theme-aware color functions
common/components/*.scss Updates reusable component styling with dark-theme-aware color functions
common/admin/*.scss Updates admin interface styling with dark-theme-aware color functions
embed.css.scss Updates embedded content styling with dark-theme-aware color functions

Comment on lines +497 to +503
color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));
vertical-align: inherit;
}
.title {
display: inline-block;
margin-top: 5px;
color: scale-color($primary, $lightness: 50%);
color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));

Copilot AI Jul 26, 2025

Copy link

Choose a reason for hiding this comment

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

The original code used $lightness: 30% but the replacement uses $lightness: 50% for both light and dark themes. This changes the original styling behavior and may not provide the intended visual contrast.

Copilot uses AI. Check for mistakes.
Comment on lines +522 to +527
color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));
}
.title {
display: inline-block;
margin-top: 5px;
color: scale-color($primary, $lightness: 50%);
color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));

Copilot AI Jul 26, 2025

Copy link

Choose a reason for hiding this comment

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

The original code used $lightness: 30% but the replacement uses $lightness: 50% for both light and dark themes. This changes the original styling behavior and may not provide the intended visual contrast.

Copilot uses AI. Check for mistakes.
margin-bottom: 4px;
margin-top: 0;
color: scale-color($primary, $lightness: 20%);
color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));

Copilot AI Jul 26, 2025

Copy link

Choose a reason for hiding this comment

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

The original code used $lightness: 20% but the replacement uses $lightness: 50% for the light theme. This significantly changes the color darkness and may affect visual hierarchy.

Suggested change
color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));
color: dark-light-choose(scale-color($primary, $lightness: 20%), scale-color($secondary, $lightness: 20%));

Copilot uses AI. Check for mistakes.
a {
font-weight: bold;
color: scale-color($primary, $lightness: 30%);
color: dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%));

Copilot AI Jul 26, 2025

Copy link

Choose a reason for hiding this comment

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

The original code used $lightness: 30% but the replacement uses $lightness: 70% for the light theme. This inverts the intended darkness level and may significantly affect readability.

Suggested change
color: dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%));
color: dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 30%));

Copilot uses AI. Check for mistakes.
.custom-message-length {
margin: -10px 0 10px 20px;
color: scale-color($primary, $lightness: 70%);
color: dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 70%));

Copilot AI Jul 26, 2025

Copy link

Choose a reason for hiding this comment

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

The original code used $lightness: 70% but the replacement uses $lightness: 30% for the light theme. This makes the text significantly darker than intended.

Suggested change
color: dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 70%));
color: dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 70%));

Copilot uses AI. Check for mistakes.
@github-actions

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has been open for 60 days with no activity. To keep it open, remove the stale tag, push code, or add a comment. Otherwise, it will be closed in 14 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants