Skip to content

chore: WDS outline buttons color adjustment#34615

Merged
KelvinOm merged 4 commits intoreleasefrom
chore/wds-adjust-outline-button-colors
Jul 22, 2024
Merged

chore: WDS outline buttons color adjustment#34615
KelvinOm merged 4 commits intoreleasefrom
chore/wds-adjust-outline-button-colors

Conversation

@ichik
Copy link
Contributor

@ichik ichik commented Jul 1, 2024

Description

Fixes #34336

It was rare before, but with some lighter seeds borders could be perceivably more prominent than fg. I made sure this can't be the case.

Borders are now lighter and less saturated than accents to emphasize the text label.

Before After
before-light after-light
before-dark after-dark

Automation

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

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9889245682
Commit: b1769eb
Cypress dashboard.
Tags: @tag.Anvil
Spec:


Thu, 11 Jul 2024 10:09:49 UTC

Communication

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

  • Yes
  • No

Summary by CodeRabbit

  • New Features

    • Enhanced snapshot testing with a new comparisonMethod parameter for improved image comparison in AnvilSnapshot.
  • Improvements

    • Adjusted color calculations in Light Mode and Dark Mode themes for better contrast and visual consistency.
  • Tests

    • Updated color tests to align with new RGB values reflecting changes in lightness, chroma, and hue for both Light Mode and Dark Mode themes.

@ichik ichik self-assigned this Jul 1, 2024
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 1, 2024

Walkthrough

The recent changes focused on enhancing the color contrast and prominence of text and borders in outlined buttons within the LightModeTheme and DarkModeTheme classes. Additionally, the AnvilSnapshot class in Cypress tests has been updated to include a new parameter for the image comparison method.

Changes

File Path Change Summary
.../theming/src/color/src/LightModeTheme.ts Adjustments to bdAccent, bdFocus, bdSurface, bdPositive, bdNegative, and bdWarning properties for better color contrast.
.../theming/src/color/src/DarkModeTheme.ts Modifications to bdAccent, bdNeutral, bdPositive, bdNegative, and bdWarning based on specific conditions.
.../theming/src/color/tests/LightModeTheme.test.ts Updated color tests to reflect changes in RGB color values for different properties.
.../theming/src/color/tests/DarkModeTheme.test.ts Revised RGB color values in tests for bdAccent, bdFocus, bdNeutral, bdPositive, bdNegative, and bdWarning.
.../support/Pages/Anvil/AnvilSnapshot.ts Added comparisonMethod parameter set to "ssim" in matchImageSnapshot method.

Assessment against linked issues

Objective Addressed Explanation
Adjust prominence of text and border in outlined buttons (#34336)

Poem

In the world of themes with colors so bright,
We tweaked our hues, from dawn to night.
Outlined buttons now shine with grace,
As text and borders find their place.
With Cypress snapping shots so keen,
Our UI's sharper than it's ever been. 🌈🐰


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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 Anvil Pod Issue related to Anvil project Task A simple Todo WDS team labels Jul 1, 2024
@ichik ichik added the ok-to-test Required label for CI label Jul 1, 2024
@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Jul 1, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between dc99eb1 and 8098bf3.

Files selected for processing (1)
  • app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts (1 hunks)
Additional comments not posted (2)
app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts (2)

957-957: LGTM! The luminance adjustment ensures better visibility.

The luminance value has been increased to 0.55, which makes the border color more prominent, addressing the issue of border visibility.


962-962: LGTM! The luminance adjustment ensures better visibility.

The luminance value has been increased to 0.55, which makes the border color more prominent, addressing the issue of border visibility.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8098bf3 and d248814.

Files selected for processing (1)
  • app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts

KelvinOm
KelvinOm previously approved these changes Jul 2, 2024
@vinay-appsmith
Copy link
Contributor

@ichik
The button text still isn't significantly different in value in relation to the outline, after the adjustments.
I'd propose to decrease the saturation and bump up the brightness of the outline to bring emphasis to the text.

Here is my proposal:
image

@ichik ichik marked this pull request as draft July 4, 2024 10:09
@ichik ichik dismissed KelvinOm’s stale review July 4, 2024 10:12

Going to iterate more on this

@ichik ichik removed the request for review from tbrizitsky July 4, 2024 10:12
@ichik ichik force-pushed the chore/wds-adjust-outline-button-colors branch from d248814 to 0ec743c Compare July 10, 2024 14:10
@ichik ichik requested review from KelvinOm and tbrizitsky July 10, 2024 14:14
@ichik ichik marked this pull request as ready for review July 10, 2024 14:15
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d248814 and 0ec743c.

Files selected for processing (4)
  • app/client/packages/design-system/theming/src/color/src/DarkModeTheme.ts (8 hunks)
  • app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts (9 hunks)
  • app/client/packages/design-system/theming/src/color/tests/DarkModeTheme.test.ts (5 hunks)
  • app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts (6 hunks)
Additional comments not posted (27)
app/client/packages/design-system/theming/src/color/tests/DarkModeTheme.test.ts (5)

730-735: Verify the updated bdAccent test cases.

The test cases for bdAccent now include checks for chroma values less than and greater than 0.04. Ensure that these values are consistent with the changes in the DarkModeTheme class.

Verification successful

Verification successful for bdAccent test cases.

The test cases for bdAccent in DarkModeTheme.test.ts correctly reflect the behavior of the bdAccent getter in the DarkModeTheme class, validating the expected RGB values based on the chroma levels.

  • app/client/packages/design-system/theming/src/color/src/DarkModeTheme.ts: Implementation of bdAccent getter confirms the chroma and lightness adjustments.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of the `bdAccent` getter in the `DarkModeTheme` class.

# Test: Search for the `bdAccent` getter implementation. Expect: Correct handling of chroma values.
ast-grep --lang typescript --pattern $'private get bdAccent() {
  $$$
}'

Length of output: 528


Script:

#!/bin/bash
# Description: Verify the implementation of the `bdAccent` getter in the `DarkModeTheme` class.

# Test: Search for the `bdAccent` getter implementation. Expect: Correct handling of chroma values.
ast-grep --lang typescript --pattern $'class DarkModeTheme {
  $$$
  private get bdAccent() {
    $$$
  }
  $$$
}'

Length of output: 115


Script:

#!/bin/bash
# Description: Search for the `bdAccent` keyword across the codebase to verify its implementation.

# Test: Search for the `bdAccent` keyword.
rg 'bdAccent' -A 5

Length of output: 17842


769-769: Verify the updated bdNeutral test case.

The test case for bdNeutral now includes a check for chroma values less than 0.04. Ensure that this value is consistent with the changes in the DarkModeTheme class.

Verification successful

Verify the updated bdNeutral test case.

The bdNeutral getter in the DarkModeTheme class correctly handles chroma values, setting it to 0.012 and adjusting to 0 for achromatic seeds. The test case expecting "rgb(33.384% 33.384% 33.384%)" is consistent with this implementation.

  • app/client/packages/design-system/theming/src/color/src/DarkModeTheme.ts:
    • private get bdNeutral() { ... }
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of the `bdNeutral` getter in the `DarkModeTheme` class.

# Test: Search for the `bdNeutral` getter implementation. Expect: Correct handling of chroma values.
ast-grep --lang typescript --pattern $'private get bdNeutral() {
  $$$
}'

Length of output: 530


Script:

#!/bin/bash
# Description: Verify the implementation of the `bdNeutral` getter in the `DarkModeTheme` class.

# Test: Search for the `bdNeutral` getter implementation. Expect: Correct handling of chroma values.
ast-grep --lang typescript --pattern 'private get bdNeutral() {
  return $_;
}'

Length of output: 544


Script:

#!/bin/bash
# Description: Verify the implementation of the `bdNeutral` getter in the `DarkModeTheme` class.

# Search for the `bdNeutral` getter implementation in TypeScript files.
rg 'private get bdNeutral' -A 10 --glob '**/*.ts'

Length of output: 4429


778-778: Verify the updated bdNeutralHover test case.

The test case for bdNeutralHover now includes a check for the color value. Ensure that this value is consistent with the changes in the DarkModeTheme class.


785-785: Verify the updated bdPositive test case.

The test case for bdPositive now includes a check for the color value. Ensure that this value is consistent with the changes in the DarkModeTheme class.

Verification successful

Verify the updated bdPositive test case.

The bdPositive getter in the DarkModeTheme class correctly handles color values by cloning bgPositive and setting specific properties. This implementation aligns with the expected RGB values in the test case.

  • Implementation Location:
    • app/client/packages/design-system/theming/src/color/src/DarkModeTheme.ts: private get bdPositive()
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of the `bdPositive` getter in the `DarkModeTheme` class.

# Test: Search for the `bdPositive` getter implementation. Expect: Correct handling of color values.
ast-grep --lang typescript --pattern $'private get bdPositive() {
  $$$
}'

Length of output: 532


Script:

#!/bin/bash
# Description: Verify the implementation of the `bdPositive` getter in the `DarkModeTheme` class.

# Test: Search for the `bdPositive` getter implementation. Expect: Correct handling of color values.
ast-grep --lang typescript --pattern 'get bdPositive() {
  $$$
}'

Length of output: 516


Script:

#!/bin/bash
# Description: Verify the implementation of the `bdPositive` getter in the `DarkModeTheme` class.

# Test: Search for the `bdPositive` getter implementation. Expect: Correct handling of color values.
ast-grep --lang typescript --pattern 'get bdPositive() {
  return $_;
}'

Length of output: 530


Script:

#!/bin/bash
# Description: Verify the implementation of the `bdPositive` getter in the `DarkModeTheme` class.

# Test: Search for the `bdPositive` getter implementation. Expect: Correct handling of color values.
rg 'get bdPositive' -A 5 --type ts

Length of output: 2516


742-762: Verify the updated bdFocus test cases.

The test cases for bdFocus now include checks for various lightness and chroma values. Ensure that these values are consistent with the changes in the DarkModeTheme class.

Verification successful

Verify the updated bdFocus test cases.

The bdFocus getter in the DarkModeTheme class adjusts the lightness to 0.8 and sets the chroma to 0.12 if it is below this value. The test cases should cover these scenarios to ensure the expected RGB values are returned.

  • Ensure that the test cases for bdFocus correctly validate the lightness adjustment to 0.8.
  • Ensure that the test cases for bdFocus correctly validate the chroma adjustment to a minimum of 0.12.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of the `bdFocus` getter in the `DarkModeTheme` class.

# Test: Search for the `bdFocus` getter implementation. Expect: Correct handling of lightness and chroma values.
ast-grep --lang typescript --pattern $'private get bdFocus() {
  $$$
}'

Length of output: 526


Script:

#!/bin/bash
# Description: Verify the implementation of the `bdFocus` getter in the `DarkModeTheme` class.

# Search for the `bdFocus` getter implementation in the codebase.
rg 'bdFocus' -A 10

Length of output: 14227

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

782-782: Verify the correctness of the expected RGB value.

The expected RGB value for the bdFocus color when lightness < 0.6 has been updated.

Ensure the new RGB value matches the intended color adjustment.


787-787: Verify the correctness of the expected RGB value.

The expected RGB value for the bdFocus color when lightness > 0.8 has been updated.

Ensure the new RGB value matches the intended color adjustment.


792-792: Verify the correctness of the expected RGB value.

The expected RGB value for the bdFocus color when chroma < 0.15 has been updated.

Ensure the new RGB value matches the intended color adjustment.


797-797: Verify the correctness of the expected RGB value.

The expected RGB value for the bdFocus color when hue is between 0 and 55 has been updated.

Ensure the new RGB value matches the intended color adjustment.


802-802: Verify the correctness of the expected RGB value.

The expected RGB value for the bdFocus color when hue > 340 has been updated.

Ensure the new RGB value matches the intended color adjustment.


827-827: Verify the correctness of the expected RGB value.

The expected RGB value for the bdPositive color has been updated.

Ensure the new RGB value matches the intended color adjustment.


836-836: Verify the correctness of the expected RGB value.

The expected RGB value for the bdPositiveHover color has been updated.

Ensure the new RGB value matches the intended color adjustment.


845-845: Verify the correctness of the expected RGB value.

The expected RGB value for the bdNegative color has been updated.

Ensure the new RGB value matches the intended color adjustment.


854-854: Verify the correctness of the expected RGB value.

The expected RGB value for the bdNegativeHover color has been updated.

Ensure the new RGB value matches the intended color adjustment.


861-861: Verify the correctness of the expected RGB value.

The expected RGB value for the bdWarning color has been updated.

Ensure the new RGB value matches the intended color adjustment.

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

921-936: Adjustments to bdAccent Color Values

The changes to bdAccent involve adjusting the lightness and chroma values based on whether the seed color is cold or not, and ensuring the border is lighter and less saturated to emphasize the text label.


946-947: Increase Lightness for bdFocus

The lightness value for the bdFocus color has been increased to 0.8, which should improve visibility for keyboard focus outlines.


961-969: Adjustments to bdNeutral Lightness and Chroma

The changes to bdNeutral involve setting the lightness to 0.4 and chroma to 0.012, with additional adjustments based on contrast calculations.


996-1001: Adjustments to bdPositive Lightness and Chroma

The lightness for bdPositive is set to 0.41 and chroma to 0.05, with additional adjustments based on contrast calculations.


Line range hint 1020-1043:
Adjustments to bdNegative Lightness and Chroma

The lightness for bdNegative is set to 0.46 and chroma to 0.1, with additional adjustments based on hue and contrast calculations.


Line range hint 1064-1087:
Adjustments to bdWarning Lightness and Chroma

The lightness for bdWarning is set to 0.48 and chroma to 0.12, with additional adjustments based on hue and contrast calculations.

app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts (6)

936-947: Adjustments to bdAccent Color Values

The changes to bdAccent involve adjusting the lightness to 0.55 and chroma based on whether the seed color is achromatic or not, ensuring the border is lighter and less saturated to emphasize the text label.


957-959: Increase Lightness for bdFocus

The lightness value for the bdFocus color has been increased to 0.55, which should improve visibility for keyboard focus outlines.


974-981: Adjustments to bdNeutral Chroma and Lightness

The chroma for bdNeutral is set to 0.01, with additional adjustments to lightness based on contrast calculations.


1013-1014: Adjustments to bdPositive Lightness

The lightness for bdPositive is set to 0.8, with additional adjustments based on hue and contrast calculations.


1049-1050: Adjustments to bdNegative Lightness

The lightness for bdNegative is set to 0.78, with additional adjustments based on hue and contrast calculations.


1085-1086: Adjustments to bdWarning Lightness

The lightness for bdWarning is set to 0.82, with additional adjustments based on hue and contrast calculations.

@KelvinOm KelvinOm added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jul 11, 2024
@KelvinOm KelvinOm added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jul 11, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0ec743c and b1769eb.

Files ignored due to path filters (12)
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetPreview.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetPreview.snap.png is excluded by !**/*.png
Files selected for processing (1)
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts (1 hunks)
Additional context used
Path-based instructions (1)
app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts (1)

Pattern app/client/cypress/**/**.*: Follow best practices for Cypress code and e2e automation.
Avoid using cy.wait in code.
Avoid using cy.pause in code.
Use variables for locators, not strings.
Use data-* attributes for selectors; avoid Xpaths and CSS attributes.
Avoid selectors like .btn.submit or button[type=submit].
Perform logins via API with LoginFromAPI.
Only interact with controlled sites/servers.
Ensure tests can run with it.only and are independent.
Use before, beforeEach, after, afterEach correctly; clean state before tests.
Check new specs for flakiness by running them 10 times on CI.
Use multiple assertions; don't treat Cypress as unit tests.
Use constants for strings.
Include datasource operations in before hooks.
Do not use duplicate filenames.

Additional comments not posted (1)
app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts (1)

27-29: LGTM!

The addition of the comparisonMethod: "ssim" parameter to the matchImageSnapshot function improves the accuracy of image snapshot comparisons.

@github-actions
Copy link

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions bot added the Stale label Jul 21, 2024
@ichik ichik removed the Stale label Jul 21, 2024
@KelvinOm KelvinOm merged commit 78413ab into release Jul 22, 2024
@KelvinOm KelvinOm deleted the chore/wds-adjust-outline-button-colors branch July 22, 2024 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Anvil Pod Issue related to Anvil project ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog Task A simple Todo WDS team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: Adjust prominence of text and border in outlined buttons

4 participants