chore: Adjust prominence of icon and main text in StatBox #34729
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe recent code update simplifies the color calculation logic for Changes
Assessment against linked issues
Poem
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configration File (
|
|
@KelvinOm we need to also be able to pass the color to the icon and set it to |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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)
Files skipped from review due to trivial changes (3)
- app/client/packages/design-system/theming/src/color/src/DarkModeTheme.ts
- app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts
- app/client/packages/design-system/theming/src/color/tests/DarkModeTheme.test.ts
Additional comments not posted (2)
app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts (2)
600-612: Verify new RGB values forfgNeutralcolor calculations.The RGB values for
fgNeutralhave been updated. Ensure these new values are correctly calculated and reflect the intended color adjustments.
622-622: Verify new RGB value forfgNeutralSubtlecolor calculation.The RGB value for
fgNeutralSubtlehas been updated. Ensure this new value is correctly calculated and reflects the intended color adjustment.Verification successful
Verify new RGB value for
fgNeutralSubtlecolor calculation.The
fgNeutralSubtlecolor is derived by incrementing the lightness offgNeutral, which itself is derived fromfgwith a lightness increment. The initialfgcolor is set with a lightness of 0.12. Ensure these adjustments result in the expected RGB value ofrgb(19.892% 19.892% 19.892%).
LightModeTheme.ts:
private get fg(): Initializesfgwiththis.seedColorand setscolor.oklch.l = 0.12.private get fgNeutral(): Clonesfgand increments lightness by 0.1.private get fgNeutralSubtle(): ClonesfgNeutraland increments lightness by 0.1.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new RGB value for `fgNeutralSubtle` color calculation. # Test: Search for the `fgNeutralSubtle` color calculation in the `LightModeTheme` implementation. rg --type python $'fgNeutralSubtle'Length of output: 65
Script:
#!/bin/bash # Description: Verify the new RGB value for `fgNeutralSubtle` color calculation. # Test: Search for the `fgNeutralSubtle` color calculation in the `LightModeTheme` implementation. rg 'fgNeutralSubtle' --type-add 'web:*.{ts,tsx,js,jsx}' --type webLength of output: 1461
Script:
#!/bin/bash # Description: Extract the relevant code segments for `fgNeutralSubtle` calculation in `LightModeTheme.ts`. # Extract the definition of the `fgNeutralSubtle` getter in `LightModeTheme.ts`. rg --pcre2 --context 5 'private get fgNeutralSubtle' app/client/packages/design-system/theming/src/color/src/LightModeTheme.tsLength of output: 300
Script:
#!/bin/bash # Description: Extract the definition of `this.fgNeutral` in `LightModeTheme.ts` to verify its initial value. # Extract the definition and assignment of `fgNeutral` in `LightModeTheme.ts`. rg --pcre2 --context 5 'fgNeutral' app/client/packages/design-system/theming/src/color/src/LightModeTheme.tsLength of output: 1027
Script:
#!/bin/bash # Description: Extract the definition of `this.fg` in `LightModeTheme.ts` to verify its initial value. # Extract the definition and assignment of `fg` in `LightModeTheme.ts`. rg --pcre2 --context 5 'fg' app/client/packages/design-system/theming/src/color/src/LightModeTheme.tsLength of output: 6066
2 tasks
KelvinOm
added a commit
that referenced
this pull request
Jul 9, 2024
## Description 1. Add scripts for local e2e testing in the docker container. 2. Add types for cypress-image-snapshot In this PR, I also added the following PRS as they affect screenshot tests as well. I did this to speed up the process and unblock the team. #34528 #34546 #34676 #34729 #34638 #34639 #34511 To run E2E tests locally in docker, you need to do the following: 1. Run FE locally and prepare the tests for local launch. See the instructions [here](https://github.com/appsmithorg/appsmith/blob/release/contributions/ClientSetup.md). 2. Run `yarn cypress:snapshot:docker:build` — this will create a docker container with the necessary environment. 3. Run `yarn cypress:snapshot:docker "./cypress/e2e/Regression/ClientSide/Anvil/Widgets/*_spec.ts" updateSnapshots=false`. Here we can use the path to a specific file, or set `updateSnapshots=true` flag to update the screenshots. ## Automation /ok-to-test tags="@tag.Anvil" ### :mag: Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9844579277> > Commit: 75f2659 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9844579277&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` > Spec: > <hr>Mon, 08 Jul 2024 18:37:36 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for image snapshot testing with the `@types/cypress-image-snapshot` dependency. - **Refactor** - Updated test specifications by removing unnecessary `triggerInputInvalidState()` calls. - Reorganized and improved efficiency of image snapshot methods for various devices. - **Chores** - Updated `Dockerfile` to configure the Cypress environment with specific versions for dependencies. - Changed import paths in `e2e.js` for better module resolution. - **Style** - Fixed a comment typo in Cypress plugin configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Pawan Kumar <[email protected]> Co-authored-by: unknown <[email protected]>
|
Merged here. |
Shivam-z
pushed a commit
to Shivam-z/appsmith
that referenced
this pull request
Jul 10, 2024
## Description 1. Add scripts for local e2e testing in the docker container. 2. Add types for cypress-image-snapshot In this PR, I also added the following PRS as they affect screenshot tests as well. I did this to speed up the process and unblock the team. appsmithorg#34528 appsmithorg#34546 appsmithorg#34676 appsmithorg#34729 appsmithorg#34638 appsmithorg#34639 appsmithorg#34511 To run E2E tests locally in docker, you need to do the following: 1. Run FE locally and prepare the tests for local launch. See the instructions [here](https://github.com/appsmithorg/appsmith/blob/release/contributions/ClientSetup.md). 2. Run `yarn cypress:snapshot:docker:build` — this will create a docker container with the necessary environment. 3. Run `yarn cypress:snapshot:docker "./cypress/e2e/Regression/ClientSide/Anvil/Widgets/*_spec.ts" updateSnapshots=false`. Here we can use the path to a specific file, or set `updateSnapshots=true` flag to update the screenshots. ## Automation /ok-to-test tags="@tag.Anvil" ### :mag: Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9844579277> > Commit: 75f2659 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9844579277&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` > Spec: > <hr>Mon, 08 Jul 2024 18:37:36 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for image snapshot testing with the `@types/cypress-image-snapshot` dependency. - **Refactor** - Updated test specifications by removing unnecessary `triggerInputInvalidState()` calls. - Reorganized and improved efficiency of image snapshot methods for various devices. - **Chores** - Updated `Dockerfile` to configure the Cypress environment with specific versions for dependencies. - Changed import paths in `e2e.js` for better module resolution. - **Style** - Fixed a comment typo in Cypress plugin configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Pawan Kumar <[email protected]> Co-authored-by: unknown <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #34334
Automation
/ok-to-test tags="@tag.Anvil"
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
fgNeutralin both Dark Mode and Light Mode themes, enhancing visual clarity.Bug Fixes
fgNeutralandfgNeutralSubtlecolors to reflect new calculations, ensuring accuracy in different scenarios.