Skip to content

fix: WDS neutral foregrounds#40037

Merged
ichik merged 1 commit intoreleasefrom
fix/wds-subtle-text-colors
Apr 2, 2025
Merged

fix: WDS neutral foregrounds#40037
ichik merged 1 commit intoreleasefrom
fix/wds-subtle-text-colors

Conversation

@ichik
Copy link
Contributor

@ichik ichik commented Apr 2, 2025

Description

Fixes brown-ish neutrals.

Automation

/ok-to-test tags="@tag.Sanity"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14222219250
Commit: 98dd30c
Cypress dashboard.
Tags: @tag.Sanity
Spec:


Wed, 02 Apr 2025 15:13:23 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • Style Changes
    • Adjusted the neutral foreground color calculations in both dark and light modes to improve contrast and visual consistency.
  • Tests
    • Updated the color verification tests to ensure the revised color adjustments perform as expected.

@ichik ichik requested a review from a team as a code owner April 2, 2025 14:29
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 2, 2025

Walkthrough

This PR modifies the color adjustment logic in the DarkModeTheme and LightModeTheme classes. For both themes, the fgNeutral and fgNeutralSubtle getters are updated by changing the lightness and chroma adjustments. In DarkModeTheme, a direct chroma reset is replaced with gradual decrements and a safeguard against negative values, while the fgNeutralSubtle lightness decrement is reduced. Similar modifications are applied in LightModeTheme with corresponding updates in the test cases to reflect the new expected RGB outputs.

Changes

Files Change Summary
app/.../DarkModeTheme.ts
app/.../tests/DarkModeTheme.test.ts
Updated fgNeutral: replaced direct chroma reset with decrement in lightness (–0.05) and chroma (–0.04) with a non-negative check; modified fgNeutralSubtle (lightness decrement from –0.3 to –0.2); updated tests with new expected RGB values.
app/.../LightModeTheme.ts
app/.../tests/LightModeTheme.test.ts
Adjusted fgNeutral: increased lightness (+0.05) and reduced chroma (–0.02) with a safeguard for negative values; changed fgNeutralSubtle lightness increase (from 0.22 to 0.12); updated tests with revised RGB expectations.

Possibly related PRs

Suggested labels

Bug, Anvil Pod, WDS team, ok-to-test

Suggested reviewers

  • KelvinOm
  • tbrizitsky
  • danielmorris-as

Poem

In the realm of dark and light,
Colors adjust with gentle might.
A tweak here, a change so neat,
Making every hue complete.
Code sings in vibrant array,
Celebrating change the smart way!


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b52f03a and 98dd30c.

📒 Files selected for processing (4)
  • app/client/packages/design-system/theming/src/color/src/DarkModeTheme.ts (1 hunks)
  • app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts (1 hunks)
  • app/client/packages/design-system/theming/src/color/tests/DarkModeTheme.test.ts (2 hunks)
  • app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts (2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts (2)
app/client/packages/design-system/theming/src/color/src/DarkModeTheme.ts (2)
  • fgNeutral (762-774)
  • fgNeutralSubtle (776-782)
app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts (3)
  • fgNeutral (766-778)
  • LightModeTheme (7-1329)
  • fgNeutralSubtle (780-786)
⏰ Context from checks skipped due to timeout of 90000ms (10)
  • GitHub Check: perform-test / client-build / client-build
  • GitHub Check: perform-test / rts-build / build
  • GitHub Check: perform-test / server-build / server-unit-tests
  • GitHub Check: client-unit-tests / client-unit-tests
  • GitHub Check: client-lint / client-lint
  • GitHub Check: client-prettier / prettier-check
  • GitHub Check: client-build / client-build
  • GitHub Check: storybook-tests
  • GitHub Check: chromatic-deployment
  • GitHub Check: chromatic-deployment
🔇 Additional comments (6)
app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts (2)

770-775: Improved neutral foreground color calculation

The changes to the fgNeutral getter adjust the lightness and apply a more gradual chroma reduction instead of directly setting it to zero. This approach prevents the brownish appearance in neutral foregrounds while maintaining a subtle color harmony with the theme.

The addition of the safeguard against negative chroma values is also a good practice.


783-783: Appropriate lightness adjustment in fgNeutralSubtle

Reducing the lightness increment from 0.22 to 0.12 provides better contrast while still maintaining the subtle nature of this color variant. This change aligns well with the modification in the fgNeutral getter.

app/client/packages/design-system/theming/src/color/src/DarkModeTheme.ts (2)

766-771: Improved neutral foreground calculation for dark mode

Similar to the light mode changes, this modification improves how neutral foreground colors are calculated in dark mode. Instead of directly zeroing out chroma, the approach now decreases lightness by 0.05 and gradually reduces chroma with a safeguard against negative values.

This change should resolve the brownish appearance issue while maintaining color harmony.


779-779: Appropriate lightness adjustment in dark mode's fgNeutralSubtle

Reducing the lightness decrement from 0.3 to 0.2 provides a better balance between contrast and subtlety for this color variant. This change is consistent with the modifications in the light mode theme.

app/client/packages/design-system/theming/src/color/tests/DarkModeTheme.test.ts (1)

641-641: Improved neutral foreground colors in DarkModeTheme

These updated test values reflect the implementation changes in the DarkModeTheme class to fix the brownish tint in neutral foregrounds. The adjustments to lightness and chroma values in the OKLCH color space result in more balanced, truly neutral colors.

Also applies to: 647-647, 653-653, 663-663

app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts (1)

673-673: Neutralized foreground colors in LightModeTheme

These test updates reflect significant improvements to the neutral foreground colors. Previously, "neutral" colors had strong blue/purple tints, which has been corrected to use true grays for achromatic inputs and more balanced tints for chromatic inputs. This change properly addresses the brownish neutral foregrounds issue mentioned in the PR description.

Also applies to: 679-679, 685-685, 695-695

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the Bug Something isn't working label Apr 2, 2025
@ichik ichik added the ok-to-test Required label for CI label Apr 2, 2025
@ichik ichik enabled auto-merge (squash) April 2, 2025 14:30
@ichik ichik merged commit 43baad9 into release Apr 2, 2025
55 of 56 checks passed
@ichik ichik deleted the fix/wds-subtle-text-colors branch April 2, 2025 15:13
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Apr 12, 2025
## Description

Fixes brown-ish neutrals.

## Automation

/ok-to-test tags="@tag.Sanity"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!CAUTION]  
> If you modify the content in this section, you are likely to disrupt
the CI result for your PR.

<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working ok-to-test Required label for CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants