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

[everettbu]

Test 7

[github-actions]

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.

[mfeuerstein]

PR Review — approved

Reviewed 30 files. 0 high-severity issues found. Verdict: approved.

app/assets/stylesheets/common/admin/admin_base.scss (low)

• Reviewed app/assets/stylesheets/common/admin/admin_base.scss — looks good

app/assets/stylesheets/common/base/_topic-list.scss (low)

• Reviewed app/assets/stylesheets/common/base/_topic-list.scss — looks good

app/assets/stylesheets/common/base/share_link.scss (low)

• Reviewed app/assets/stylesheets/common/base/share_link.scss — looks good

app/assets/stylesheets/common/base/login.scss (low)

• Reviewed app/assets/stylesheets/common/base/login.scss — looks good

app/assets/stylesheets/common/base/search.scss (low)

• Reviewed app/assets/stylesheets/common/base/search.scss — looks good

app/assets/stylesheets/common/base/notification-options.scss (low)

• Reviewed app/assets/stylesheets/common/base/notification-options.scss — looks good

app/assets/stylesheets/common/base/header.scss (low)

• Reviewed app/assets/stylesheets/common/base/header.scss — looks good

app/assets/stylesheets/common/components/badges.css.scss (low)

• Reviewed app/assets/stylesheets/common/components/badges.css.scss — looks good

app/assets/stylesheets/common/base/user.scss (low)

• Reviewed app/assets/stylesheets/common/base/user.scss — looks good

app/assets/stylesheets/common/base/user-badges.scss (low)

• Reviewed app/assets/stylesheets/common/base/user-badges.scss — looks good

app/assets/stylesheets/desktop/compose.scss (low)

• Reviewed app/assets/stylesheets/desktop/compose.scss — looks good

app/assets/stylesheets/desktop/header.scss (low)

• Reviewed app/assets/stylesheets/desktop/header.scss — looks good

app/assets/stylesheets/common/components/buttons.css.scss (low)

• Reviewed app/assets/stylesheets/common/components/buttons.css.scss — looks good

app/assets/stylesheets/desktop/login.scss (low)

• Reviewed app/assets/stylesheets/desktop/login.scss — looks good

app/assets/stylesheets/desktop/topic-list.scss (low)

• Reviewed app/assets/stylesheets/desktop/topic-list.scss — looks good

app/assets/stylesheets/desktop/topic-post.scss (low)

• Reviewed app/assets/stylesheets/desktop/topic-post.scss — looks good

app/assets/stylesheets/desktop/modal.scss (low)

• Reviewed app/assets/stylesheets/desktop/modal.scss — looks good

app/assets/stylesheets/common/base/discourse.scss (low)

• Reviewed app/assets/stylesheets/common/base/discourse.scss — looks good

app/assets/stylesheets/desktop/topic.scss (low)

• Reviewed app/assets/stylesheets/desktop/topic.scss — looks good

app/assets/stylesheets/embed.css.scss (low)

• Reviewed app/assets/stylesheets/embed.css.scss — looks good

app/assets/stylesheets/common/base/topic-post.scss (low)

• Reviewed app/assets/stylesheets/common/base/topic-post.scss — looks good

app/assets/stylesheets/mobile/compose.scss (low)

• Reviewed app/assets/stylesheets/mobile/compose.scss — looks good

app/assets/stylesheets/desktop/user.scss (low)

• Reviewed app/assets/stylesheets/desktop/user.scss — looks good

app/assets/stylesheets/mobile/login.scss (low)

• Reviewed app/assets/stylesheets/mobile/login.scss — looks good

app/assets/stylesheets/desktop/queued-posts.scss (low)

• Reviewed app/assets/stylesheets/desktop/queued-posts.scss — looks good

app/assets/stylesheets/desktop/upload.scss (low)

• Reviewed app/assets/stylesheets/desktop/upload.scss — looks good

app/assets/stylesheets/mobile/topic.scss (low)

• Reviewed app/assets/stylesheets/mobile/topic.scss — looks good

app/assets/stylesheets/mobile/modal.scss (low)

• Reviewed app/assets/stylesheets/mobile/modal.scss — looks good

app/assets/stylesheets/mobile/topic-list.scss (low)

• Reviewed app/assets/stylesheets/mobile/topic-list.scss — looks good

app/assets/stylesheets/mobile/topic-post.scss (low)

• Reviewed app/assets/stylesheets/mobile/topic-post.scss — looks good

[zach-source]

Incorrect dark-light-choose lightness values — see inline.

[ron-x5labs]

Code Review: scale-color $lightness must use $secondary for dark themes

Problem

Discourse muted-text colors were defined as scale-color($primary, $lightness: N%), which only renders correctly in light color schemes. In dark themes $primary is light, so scaling it lighter makes muted text near-invisible. The PR wraps each value in dark-light-choose(...) to pick a $secondary-based color in dark themes.

Solution Reviewed

~95 replacements across 32 SCSS files convert scale-color($primary, $lightness: N%) → dark-light-choose(scale-color($primary, $lightness: N%), scale-color($secondary, $lightness: (100-N)%)). The helper (common/foundation/variables.scss:46) returns arg 1 when brightness($primary) < brightness($secondary) (light theme) and arg 2 otherwise (dark theme), so arg 1 must byte-match the original to preserve light-theme appearance, and arg 2 is the complement for dark theme.

Summary

The refactor is mechanically sound and the convention is right almost everywhere, but 5 lines changed the first (light-theme) argument's lightness away from the original — a visible regression for existing light-theme users — and consequently mis-paired the dark-theme complement. One additional bare scale-color($primary) was left unconverted in a file otherwise fully converted. Recommend changes before merge.

Verification

• SCSS build/typecheck — skipped: no Ruby/Sass toolchain configured in the review worktree; findings are static-value comparisons against the original removed lines and the helper contract, which are deterministic without compilation.
• dark-light-choose / dark-light-diff definitions confirmed at common/foundation/variables.scss:39-52.
• Cross-file counterparts compared (desktop vs mobile, common/base vs desktop/mobile) to confirm divergences.

Issues Found

Blocking

• desktop/topic-post.scss:291 — .topic-meta-data h5 a: original scale-color($primary, $lightness: 30%) → dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%)). Both args swapped: arg 1 (light theme) must be 30% to preserve the existing muted-link color; arg 2 (dark theme) must be 70% (complement of 30). Light theme now renders 70% (was 30%) — visible regression.
• mobile/modal.scss:102 — .custom-message-length: original scale-color($primary, $lightness: 70%) → dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 70%)). Both swapped: arg 1 must be 70%, arg 2 must be 30%. Light theme now 30% (was 70%); inconsistent with the correct desktop/modal.scss:94 counterpart (70%/30%).
• mobile/topic-post.scss:182 — a.star h3: original scale-color($primary, $lightness: 20%) → dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)). Arg 1 must be 20%, arg 2 must be 80% (complement). Light theme now 50% (was 20%); inconsistent with desktop/topic-post.scss:321 h3 which correctly uses 20%/80%.
• desktop/user.scss:522 — .group-member-info .name: original scale-color($primary, $lightness: 30%) → dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)). Arg 1 must be 30%, arg 2 must be 70%. Light theme now 50% (was 30%); diverges from common/base/user.scss:124 .name which correctly uses 30%/70%.
• mobile/user.scss:497 — .group-member-info .name: same bug as desktop/user.scss:522. Original 30% → 50%/50%; should be 30%/70%. Light theme regresses 30%→50%.

Non-blocking

• common/base/_topic-list.scss:115 — .topic-list.categories .category .badge-notification still uses bare scale-color($primary, $lightness: 50%) while the PR converted identical 50% calls on lines 48/51/55/69 of the same file. This element stays light-theme-only; wrap it for consistency: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)).

Verdict

Recommend changes before merge — 5 lines regress the existing light-theme appearance and mis-pair the dark-theme complement; all are one-arg lightness fixes.

[ron-x5labs]

Blocking — light-theme regression + wrong dark-theme complement.

Original: scale-color($primary, $lightness: 30%)
New: dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%))

dark-light-choose($light-theme-result, $dark-theme-result) returns arg 1 in the light theme, so arg 1 must equal the original 30% to preserve the existing muted-link color. Both args are swapped here — arg 1 should be 30% and arg 2 should be 70% (the complement of 30). As written, light-theme users see a 70%-lightness link (was 30%), and the dark-theme value is too dark.

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

[ron-x5labs]

Blocking — light-theme regression + wrong dark-theme complement.

Original: scale-color($primary, $lightness: 70%)
New: dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 70%))

Both args are swapped. Arg 1 (light theme) must be 70% to preserve the original; arg 2 (dark theme) must be 30% (complement of 70). The desktop counterpart desktop/modal.scss:94 correctly uses 70%/30%.

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

[ron-x5labs]

Blocking — light-theme regression + wrong dark-theme complement.

Original: scale-color($primary, $lightness: 20%)
New: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%))

Arg 1 (light theme) must be 20% to preserve the original h3 color; arg 2 (dark theme) must be 80% (complement of 20). The desktop counterpart desktop/topic-post.scss:321 correctly uses 20%/80%, and the adjacent h4 below (line 190) correctly keeps 50%.

Fix: dark-light-choose(scale-color($primary, $lightness: 20%), scale-color($secondary, $lightness: 80%))

[ron-x5labs]

Blocking — light-theme regression + wrong dark-theme complement.

Original: scale-color($primary, $lightness: 30%)
New: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%))

Arg 1 (light theme) must be 30% to preserve the original name color; arg 2 (dark theme) must be 70% (complement of 30). The shared rule common/base/user.scss:124 .name correctly uses 30%/70%.

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

[ron-x5labs]

Blocking — light-theme regression + wrong dark-theme complement.

Original: scale-color($primary, $lightness: 30%)
New: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%))

Same bug as desktop/user.scss:522. Arg 1 (light theme) must be 30%; arg 2 (dark theme) must be 70% (complement of 30). The shared rule common/base/user.scss:124 .name correctly uses 30%/70%.

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

[ron-x5labs]

Code Review: theme-color-scheme-pre

Problem

This PR converts hardcoded scale-color($primary, $lightness: N%) color values across 32 SCSS files to dark-light-choose(...) so a dark color scheme can render correctly. dark-light-choose($light-theme-result, $dark-theme-result) returns its first argument in light theme (original behavior) and its second in dark theme.

Solution Reviewed

Mechanical refactor of 115 scale-color($primary, ...) call sites. The correct conversion preserves the original primary lightness N in the first argument and uses the complement 100-N on $secondary in the second, e.g. scale-color($primary, $lightness: 30%) → dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 70%)).

Summary

115 of 115 conversions were applied, but a deterministic pass over every hunk found 5 where the first argument's $primary lightness was changed instead of preserved. These are logic errors: in light theme (the default) the rendered color no longer matches the pre-PR value, and in two cases the light/dark arguments are swapped. Recommend changes before merge.

Files Reviewed

All 32 changed SCSS files reviewed via deterministic hunk comparison (removed scale-color($primary, $lightness: N%) paired in-order with added dark-light-choose(...) within each @@ hunk). 115 conversions verified; 5 violations found. Risk: low (visual only) but correctness regression in the default theme.

Verification

• git diff <base>..HEAD hunk-aware script pairing each removed scale-color($primary, $lightness: N%) with its added replacement — found 5 first-argument mismatches; 0 missed conversions (115 removed == 115 added).
• Cross-reference: common/base/user.scss:124 (.name) correctly keeps 30%/70%; desktop/user.scss:522 and mobile/user.scss:497 (same .name selector) wrongly use 50%/50%.
• Cross-reference: desktop/modal.scss:94 (.custom-message-length) correctly uses 70%/30%; mobile/modal.scss:102 (same selector) has them swapped to 30%/70%.

Note on prior review

A prior pr-local-code-review run (review 4918880646) flagged these same 5 lines. The code at each line is unchanged, so all 5 findings are persistent.

Verdict

Recommend changes before merge — 5 light-theme color regressions from incorrect dark-light-choose arguments.

[ron-x5labs]

Blocking — light-theme regression (arguments swapped).

Original: scale-color($primary, $lightness: 30%)
New: dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%))

dark-light-choose returns its first argument in light theme, so the first arg must keep the original lightness (30%). Here it was changed to 70%, and the args are effectively swapped — light theme now renders the value intended for dark theme. Fix:

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

[ron-x5labs]

Blocking — light-theme regression (arguments swapped).

Original: scale-color($primary, $lightness: 70%)
New: dark-light-choose(scale-color($primary, $lightness: 30%), scale-color($secondary, $lightness: 70%))

The first argument (light-theme value) must keep the original 70%; it was changed to 30%, swapping the light/dark values. The sibling desktop/modal.scss:94 for the same .custom-message-length selector correctly uses 70% / 30% — this mobile one should match it:

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

[ron-x5labs]

Blocking — light-theme regression.

Original: scale-color($primary, $lightness: 20%)
New: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%))

The first argument must preserve the original 20%; it was changed to 50%, making this h3 noticeably lighter than before in the default theme. Fix:

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

[ron-x5labs]

Blocking — light-theme regression.

Original: scale-color($primary, $lightness: 30%)
New: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%))

The .group-member-info .name first argument must keep the original 30%; it was changed to 50%. The same .name selector in common/base/user.scss:124 was correctly converted to 30% / 70% — this should match it:

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

[ron-x5labs]

Blocking — light-theme regression.

Original: scale-color($primary, $lightness: 30%)
New: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%))

Same issue as desktop/user.scss:522 for the same .group-member-info .name selector. The first argument must keep the original 30%, not 50%. common/base/user.scss:124 correctly uses 30% / 70%. Fix:

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

[ron-x5labs]

Code Review: dark-mode color scheme retrofit

Problem

This PR retrofits the existing light-theme palette to also support a dark color scheme by wrapping each scale-color($primary, $lightness: X%) declaration in dark-light-choose(...) so a distinct dark-theme value can be chosen.

Solution Reviewed

115 declarations across 32 SCSS files were mechanically rewritten from scale-color($primary, $lightness: X%) to dark-light-choose(scale-color($primary, $lightness: X%), scale-color($secondary, $lightness: (100-X)%)). Per variables.scss:46, dark-light-choose($light-theme-result, $dark-theme-result) returns its first argument in light theme and its second in dark theme — so arg 1 must reproduce the original lightness and arg 2 must be the 100-X complement on $secondary. 110 of 115 lines follow that rule correctly; 5 do not.

Summary

The refactor direction is sound, but 5 lines carry incorrect arguments that regress the existing light-theme appearance and/or pair a wrong dark-theme complement. Each is a single-value fix; once corrected the PR is safe to merge.

Files Reviewed

• All 32 changed SCSS files — deeply reviewed via programmatic verification of every one of the 115 dark-light-choose additions against the contract above.

Verification

• Programmatic audit of all 115 dark-light-choose additions vs. original lightness + 100-X complement rule — 5 mismatches (listed inline).
• dark-light-choose semantics confirmed against app/assets/stylesheets/common/foundation/variables.scss:46.

Issues Found

Blocking (5)

See inline comments — all five are incorrect dark-light-choose arguments (wrong light-theme value and/or wrong dark-theme complement).

Verdict

Recommend changes before merge — fix the 5 flagged lines (each a single lightness-value correction) so neither theme regresses.

[ron-x5labs]

Blocking — wrong dark-light-choose arguments.

Original: scale-color($primary, $lightness: 30%). New first arg is 70%, which changes the light-theme color of this reply link. The first argument is the light-theme value (variables.scss:46) and must preserve the original 30%; the second must be the 100-30 = 70% complement.

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

[ron-x5labs]

Blocking — wrong dark-light-choose arguments.

Original .group-member-info .name color was scale-color($primary, $lightness: 30%). Both args were changed to 50%, regressing the light-theme name color and giving the dark theme a non-complementary value. Arg 1 must keep 30%; arg 2 must be 70%.

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

[ron-x5labs]

Blocking — arguments swapped.

Original .custom-message-length color was scale-color($primary, $lightness: 70%). The new first arg is 30%, which regresses the light-theme appearance. Arg 1 (light theme) must stay 70%; arg 2 (dark theme) must be the 100-70 = 30% complement.

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

[ron-x5labs]

Blocking — wrong dark-light-choose arguments.

Original a.star h3 color was scale-color($primary, $lightness: 20%). Both args were changed to 50%, noticeably altering the light-theme heading color and pairing a wrong dark value. Arg 1 must keep 20%; arg 2 must be 80%.

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

[ron-x5labs]

Blocking — wrong dark-light-choose arguments.

Same bug as desktop/user.scss:522 for .group-member-info .name. Original was scale-color($primary, $lightness: 30%); both args became 50%. Arg 1 must keep 30%; arg 2 must be 70%.

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