Skip to content

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

Open
ShashankFC wants to merge 1 commit into
theme-color-scheme-prefrom
theme-color-scheme-post
Open

scale-color $lightness must use $secondary for dark themes#7
ShashankFC wants to merge 1 commit into
theme-color-scheme-prefrom
theme-color-scheme-post

Conversation

@ShashankFC

Copy link
Copy Markdown
Collaborator

@GitHoobar

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d38c4d5f74

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 289 to +291
a {
font-weight: bold;
color: scale-color($primary, $lightness: 30%);
color: dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep light theme color for topic meta links

In light theme, dark-light-choose uses its first argument, so this change now renders topic meta links with scale-color($primary, $lightness: 70%) instead of the previous 30%. That makes the link text much lighter in the light theme, which is a regression unrelated to the dark-theme swap and can reduce readability. Consider keeping the original light-theme value and only switching the dark-theme branch to $secondary.

Useful? React with 👍 / 👎.

Comment on lines 100 to 103
.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%));
font-size: 85%;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve light-theme hint color in mobile modal

Because dark-light-choose selects the first argument in light mode, this change makes .custom-message-length use scale-color($primary, $lightness: 30%) where it previously used 70%. That substantially darkens the hint text in the light theme and is inconsistent with the stated goal of only adjusting dark-theme colors. Keep the original light-theme value and only update the dark-theme branch.

Useful? React with 👍 / 👎.

Comment on lines 179 to 183
h3 {
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%));
line-height: 23px;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid unintended light-theme change for topic map h3

In light mode, this now uses scale-color($primary, $lightness: 50%) instead of the previous 20%, so the topic map heading becomes much lighter and less prominent in the light theme. That appears to be an accidental regression while adding dark-theme support. The first argument should stay at the original 20% and only the dark-theme branch should switch to $secondary.

Useful? React with 👍 / 👎.

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