scale-color $lightness must use $secondary for dark themes#5
scale-color $lightness must use $secondary for dark themes#5akshayutture-augment wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the color styling system across the application to ensure proper theme support for both light and dark modes. The changes wrap scale-color() functions with dark-light-choose() to provide appropriate color values for each theme, using $primary for light themes and $secondary for dark themes.
Key changes:
- Replaced single
scale-color($primary, $lightness: X%)calls withdark-light-choose(scale-color($primary, $lightness: X%), scale-color($secondary, $lightness: Y%)) - Adjusted lightness percentages for dark theme colors to maintain visual consistency (complementary percentages that sum to 100%)
- Applied changes consistently across mobile, desktop, and common stylesheets
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/assets/stylesheets/mobile/user.scss | Updated color definitions for instructions, icons, and member info elements |
| app/assets/stylesheets/mobile/upload.scss | Updated description text color |
| app/assets/stylesheets/mobile/topic.scss | Updated private message glyph color |
| app/assets/stylesheets/mobile/topic-post.scss | Updated colors for buttons, post metadata, links, and user elements |
| app/assets/stylesheets/mobile/topic-list.scss | Updated topic list table colors and featured topic link colors |
| app/assets/stylesheets/mobile/modal.scss | Updated custom message length color |
| app/assets/stylesheets/mobile/login.scss | Updated login form link and instruction colors |
| app/assets/stylesheets/mobile/compose.scss | Updated draft status, close button, and disabled input colors |
| app/assets/stylesheets/embed.css.scss | Updated new user link color |
| app/assets/stylesheets/desktop/user.scss | Updated instruction and group member colors |
| app/assets/stylesheets/desktop/upload.scss | Updated description and hint text colors |
| app/assets/stylesheets/desktop/topic.scss | Updated private message glyph and reply-new link colors |
| app/assets/stylesheets/desktop/topic-post.scss | Updated numerous post control, metadata, and interaction element colors |
| app/assets/stylesheets/desktop/topic-list.scss | Updated topic list icon and link colors |
| app/assets/stylesheets/desktop/queued-posts.scss | Updated queued post metadata color |
| app/assets/stylesheets/desktop/modal.scss | Updated modal footer hint and close button colors |
| app/assets/stylesheets/desktop/login.scss | Updated login form link and instruction colors |
| app/assets/stylesheets/desktop/header.scss | Updated search result and highlight colors |
| app/assets/stylesheets/desktop/compose.scss | Updated compose icon color |
| app/assets/stylesheets/common/components/buttons.css.scss | Updated disabled button hover color |
| app/assets/stylesheets/common/components/badges.css.scss | Updated badge background and link colors |
| app/assets/stylesheets/common/base/user.scss | Updated user profile element colors |
| app/assets/stylesheets/common/base/user-badges.scss | Updated badge count and grant count colors |
| app/assets/stylesheets/common/base/topic-post.scss | Updated post metadata, quote, and status colors |
| app/assets/stylesheets/common/base/share_link.scss | Updated date display color |
| app/assets/stylesheets/common/base/search.scss | Updated search result and highlight colors |
| app/assets/stylesheets/common/base/notification-options.scss | Updated muted notification icon color |
| app/assets/stylesheets/common/base/login.scss | Updated login form text and button colors |
| app/assets/stylesheets/common/base/header.scss | Updated header notification and status colors |
| app/assets/stylesheets/common/base/discourse.scss | Updated coldmap color intensity classes |
| app/assets/stylesheets/common/base/_topic-list.scss | Updated topic list table header, cell, and excerpt colors |
| app/assets/stylesheets/common/admin/admin_base.scss | Updated admin interface colors for descriptions, borders, and filters |
| .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%)); |
There was a problem hiding this comment.
The lightness values here (30% for primary, 70% for secondary) differ from the pattern used throughout the rest of the PR where the original lightness value was 70%. This should be dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%)) to maintain consistency with the established pattern.
| 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: 30%)); |
| a { | ||
| font-weight: bold; | ||
| color: scale-color($primary, $lightness: 30%); | ||
| color: dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%)); |
There was a problem hiding this comment.
The lightness values here (70% for primary, 30% for secondary) are inverted from the original 30% value. This should be dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 70%)) to maintain the correct complementary relationship where the percentages sum to 100%.
| 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: 70%)); |
Test 7
Replicated from ai-code-review-evaluation/discourse-copilot#7