scale-color $lightness must use $secondary for dark themes#4
Conversation
|
@cubic-dev-ai review this pull request |
@ShashankFC I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
4 issues found across 32 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/assets/stylesheets/mobile/user.scss">
<violation number="1" location="app/assets/stylesheets/mobile/user.scss:115">
P2: Unintended light-theme change: the original `$lightness` was `30%` but the `dark-light-choose` light-theme argument uses `50%`. This makes the `.group-member-info .name` text noticeably lighter in light themes compared to before. The first argument should preserve the original `30%` value.</violation>
</file>
<file name="app/assets/stylesheets/desktop/topic-post.scss">
<violation number="1" location="app/assets/stylesheets/desktop/topic-post.scss:291">
P2: Light-theme lightness value changed from 30% to 70%, which will make this text significantly lighter (more faded) on light themes. The values appear to be swapped — the original 30% should stay with `$primary` (light theme), and 70% should go to `$secondary` (dark theme).</violation>
</file>
<file name="app/assets/stylesheets/desktop/user.scss">
<violation number="1" location="app/assets/stylesheets/desktop/user.scss:59">
P2: The light-theme lightness value for `.group-member-info .name` was changed from 30% to 50%, which appears unintentional. The original 30% made the name darker/more prominent than the `.title` (which uses 50%). This PR should only be adding dark-theme support, not altering light-theme appearance. The first argument to `dark-light-choose` (light theme) should preserve the original 30% value.</violation>
</file>
<file name="app/assets/stylesheets/mobile/topic-post.scss">
<violation number="1" location="app/assets/stylesheets/mobile/topic-post.scss:49">
P2: The light-theme lightness for `.topic-map h3` was unintentionally changed from `20%` to `50%`. The PR goal is to fix dark theme colors, but this also alters the light theme appearance — making the heading noticeably lighter than intended. This looks like a copy-paste error from the `h4` rule below. The first argument should preserve the original `$lightness: 20%`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| .instructions { | ||
| color: scale-color($primary, $lightness: 50%); | ||
| color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)); |
There was a problem hiding this comment.
P2: Unintended light-theme change: the original $lightness was 30% but the dark-light-choose light-theme argument uses 50%. This makes the .group-member-info .name text noticeably lighter in light themes compared to before. The first argument should preserve the original 30% value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/assets/stylesheets/mobile/user.scss, line 115:
<comment>Unintended light-theme change: the original `$lightness` was `30%` but the `dark-light-choose` light-theme argument uses `50%`. This makes the `.group-member-info .name` text noticeably lighter in light themes compared to before. The first argument should preserve the original `30%` value.</comment>
<file context>
@@ -112,7 +112,7 @@
}
.instructions {
- color: scale-color($primary, $lightness: 50%);
+ color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));
margin-top: 5px;
margin-bottom: 10px;
</file context>
| color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)); | |
| color: dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 50%)); |
| 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.
P2: Light-theme lightness value changed from 30% to 70%, which will make this text significantly lighter (more faded) on light themes. The values appear to be swapped — the original 30% should stay with $primary (light theme), and 70% should go to $secondary (dark theme).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/assets/stylesheets/desktop/topic-post.scss, line 291:
<comment>Light-theme lightness value changed from 30% to 70%, which will make this text significantly lighter (more faded) on light themes. The values appear to be swapped — the original 30% should stay with `$primary` (light theme), and 70% should go to `$secondary` (dark theme).</comment>
<file context>
@@ -288,10 +288,10 @@ nav.post-controls {
a {
font-weight: bold;
- color: scale-color($primary, $lightness: 30%);
+ color: dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%));
}
}
</file context>
| 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%)); |
| } | ||
| .instructions { | ||
| color: scale-color($primary, $lightness: 50%); | ||
| color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)); |
There was a problem hiding this comment.
P2: The light-theme lightness value for .group-member-info .name was changed from 30% to 50%, which appears unintentional. The original 30% made the name darker/more prominent than the .title (which uses 50%). This PR should only be adding dark-theme support, not altering light-theme appearance. The first argument to dark-light-choose (light theme) should preserve the original 30% value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/assets/stylesheets/desktop/user.scss, line 59:
<comment>The light-theme lightness value for `.group-member-info .name` was changed from 30% to 50%, which appears unintentional. The original 30% made the name darker/more prominent than the `.title` (which uses 50%). This PR should only be adding dark-theme support, not altering light-theme appearance. The first argument to `dark-light-choose` (light theme) should preserve the original 30% value.</comment>
<file context>
@@ -56,7 +56,7 @@
}
.instructions {
- color: scale-color($primary, $lightness: 50%);
+ color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));
margin-left: 160px;
margin-top: 5px;
</file context>
| color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)); | |
| color: dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 50%)); |
| vertical-align: top; | ||
| background: transparent; | ||
| color: scale-color($primary, $lightness: 50%); | ||
| color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)); |
There was a problem hiding this comment.
P2: The light-theme lightness for .topic-map h3 was unintentionally changed from 20% to 50%. The PR goal is to fix dark theme colors, but this also alters the light theme appearance — making the heading noticeably lighter than intended. This looks like a copy-paste error from the h4 rule below. The first argument should preserve the original $lightness: 20%.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/assets/stylesheets/mobile/topic-post.scss, line 49:
<comment>The light-theme lightness for `.topic-map h3` was unintentionally changed from `20%` to `50%`. The PR goal is to fix dark theme colors, but this also alters the light theme appearance — making the heading noticeably lighter than intended. This looks like a copy-paste error from the `h4` rule below. The first argument should preserve the original `$lightness: 20%`.</comment>
<file context>
@@ -46,7 +46,7 @@ button {
vertical-align: top;
background: transparent;
- color: scale-color($primary, $lightness: 50%);
+ color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));
float: left;
&.hidden {
</file context>
| 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: 50%)); |
Test 7
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.
Replicated from ai-code-review-evaluation/discourse-coderabbit#7