-
Notifications
You must be signed in to change notification settings - Fork 6.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: use @ant-design/fast-color instead #4070
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes reflect a significant update in the color handling system of the project, transitioning from the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Theme
participant ColorProcessor
User->>Theme: Change Theme Color
Theme->>ColorProcessor: Process New Color
ColorProcessor->>ColorProcessor: Convert to RGB
ColorProcessor-->>Theme: Return Processed Color
Theme-->>User: Update UI with New Color
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 Configuration File (
|
There was a problem hiding this 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
Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
Files selected for processing (6)
- packages/@core/base/shared/package.json (1 hunks)
- packages/@core/base/shared/src/colorful/convert.test.ts (2 hunks)
- packages/@core/base/shared/src/colorful/convert.ts (3 hunks)
- packages/@core/base/shared/src/colorful/generator.ts (2 hunks)
- packages/effects/hooks/src/use-design-tokens.ts (4 hunks)
- packages/effects/layouts/src/widgets/preferences/blocks/theme/builtin.vue (2 hunks)
Additional comments not posted (17)
packages/@core/base/shared/src/colorful/generator.ts (2)
3-3
: Update import statement.The import statement now includes
Color
andconvertToHslCssVar
from theconvert
module. Ensure that theconvertToHslCssVar
function is correctly implemented in theconvert
module.
16-16
: Update color handling logic.The
TinyColor
instance has been replaced with aColor
instance. Ensure that theColor
class provides the same functionality asTinyColor
.packages/@core/base/shared/src/colorful/convert.ts (5)
1-4
: Correctly aliasFastColor
toColor
.The alias
Color
is correctly created forFastColor
. Ensure thatFastColor
is correctly imported and used throughout the file.
25-29
: EnsuretoHsl
method correctness.The
Color
class should provide atoHsl
method that returns an object witha
,h
,l
, ands
properties. Verify the correctness of this method.
47-47
: EnsureisValid
method correctness.The
Color
class should provide anisValid
method that returns a boolean indicating the validity of the color. Verify the correctness of this method.
10-14
: EnsuretoHsl
method correctness.The
Color
class should provide atoHsl
method that returns an object witha
,h
,l
, ands
properties. Verify the correctness of this method.
34-35
: EnsuretoRgbString
method correctness.The
Color
class should provide atoRgbString
method that returns a correctly formatted RGB string. Verify the correctness of this method.packages/@core/base/shared/package.json (1)
58-58
: Update dependency to@ant-design/fast-color
.The dependency on
@ctrl/tinycolor
has been replaced with@ant-design/fast-color
. Ensure that the new dependency version^2.0.5
is correct and compatible with the project.packages/@core/base/shared/src/colorful/convert.test.ts (3)
3-8
: LGTM! Import statement updated correctly.The addition of
convertToRgb
to the import statement is appropriate and aligns with the new test cases.
35-39
: LGTM! New test case for RGB conversion.The test case for converting HSL to RGB CSS variable format is clear and correct.
41-45
: LGTM! New test case for RGBA conversion.The test case for converting HSLA to RGBA CSS variable format is clear and correct.
packages/effects/layouts/src/widgets/preferences/blocks/theme/builtin.vue (3)
12-12
: LGTM! Import statement updated correctly.The addition of
Color
andconvertToHsl
to the import statement is appropriate and aligns with the changes made to the file.
25-25
: LGTM! Updated color handling logic.The use of the
Color
class to convertthemeColorPrimary
to a hex string is appropriate and aligns with the new color handling logic.
29-29
: LGTM! Simplified computed property.The simplification of the
builtinThemePresets
computed property improves readability and maintainability.packages/effects/hooks/src/use-design-tokens.ts (3)
4-4
: LGTM! Import statement updated correctly.The addition of
convertToRgb
to the import statement is appropriate and aligns with the changes made to the file.
105-105
: LGTM! Updated color conversion logic.The use of
convertToRgb
to convert HSL values to RGB is appropriate and aligns with the new color handling logic.
161-161
: LGTM! Updated color conversion logic.The use of
convertToRgb
to convert HSL values to RGB is appropriate and aligns with the new color handling logic.
There was a problem hiding this 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
Files selected for processing (2)
- packages/@core/base/shared/src/colorful/convert.test.ts (3 hunks)
- packages/@core/base/shared/src/colorful/convert.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/@core/base/shared/src/colorful/convert.test.ts
- packages/@core/base/shared/src/colorful/convert.ts
All FastColor objects are valid. So isValid is always true. FastColor("not-a-color") -> `#000000`
There was a problem hiding this 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
Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
Files selected for processing (8)
- packages/@core/base/shared/build.config.ts (1 hunks)
- packages/@core/base/shared/package.json (2 hunks)
- packages/@core/base/shared/src/color/convert.test.ts (2 hunks)
- packages/@core/base/shared/src/color/convert.ts (1 hunks)
- packages/@core/base/shared/src/color/generator.ts (2 hunks)
- packages/@core/base/shared/src/index.ts (1 hunks)
- packages/@core/ui-kit/shadcn-ui/src/components/menu-badge/menu-badge.vue (2 hunks)
- packages/utils/src/index.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/@core/base/shared/build.config.ts
- packages/@core/base/shared/src/index.ts
Files skipped from review as they are similar to previous changes (1)
- packages/@core/base/shared/package.json
Additional comments not posted (13)
packages/utils/src/index.ts (1)
2-2
: Export change detected: Ensure compatibility.The export statement has been updated from
@vben-core/shared/colorful
to@vben-core/shared/color
. Ensure that all dependent modules or components are updated to use the new export.Verification successful
Export change verified: No issues found.
The export statement has been updated from
@vben-core/shared/colorful
to@vben-core/shared/color
. No occurrences of the old export were found in the codebase, indicating that all dependent modules or components have been updated to use the new export.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usage of the old export to ensure compatibility with the new export. # Test: Search for the old export usage. Expect: No occurrences of the old export. rg --type ts -A 5 $'@vben-core/shared/colorful'Length of output: 46
Script:
#!/bin/bash # Description: Verify all usage of the old export to ensure compatibility with the new export. # Test: Search for the old export usage across all file types. Expect: No occurrences of the old export. rg '@vben-core/shared/colorful'Length of output: 31
packages/@core/base/shared/src/color/generator.ts (2)
3-3
: Import change detected: Ensure correct integration.The import statement has been updated to use
Color
from./convert
instead ofTinyColor
from@ctrl/tinycolor
. Ensure that the new class is correctly integrated and that all necessary methods are available.
16-16
: Ensure correct color conversion.The
generatorColorVariables
function now uses theColor
class for color conversion. Verify that thetoHexString
method returns the expected hexadecimal representation of the color.packages/@core/base/shared/src/color/convert.ts (4)
1-3
: NewColor
class introduced: Ensure correct usage.The
Color
class is introduced from@ant-design/fast-color
. Ensure that the class is correctly used and that all necessary methods are available.
28-32
: Ensure correct HSL CSS variable conversion.The
convertToHslCssVar
function converts a color to an HSL CSS variable. Verify that the conversion logic is correct and that the resulting string is in the expected format.Verification successful
Verified: The
convertToHslCssVar
function correctly converts a color to an HSL CSS variable string.The function implementation correctly converts the input color to HSL values and formats the string as expected.
convertToHslCssVar
function implementation found inpackages/@core/base/shared/src/color/convert.ts
:function convertToHslCssVar(color: string): string { const { a, h, l, s } = new Color(color).toHsl(); const hsl = `${Math.round(h)} ${Math.round(s * 100)}% ${Math.round(l * 100)}%`; return a < 1 ? `${hsl} / ${a}` : hsl; }Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `convertToHslCssVar` function logic. # Test: Search for the `convertToHslCssVar` function implementation. Expect: Correct conversion logic. ast-grep --lang typescript --pattern $'function convertToHslCssVar($_) { $$$ }'Length of output: 82
Script:
#!/bin/bash # Description: Verify the `convertToHslCssVar` function logic. # Test: Search for the `convertToHslCssVar` function implementation in the specified file. rg 'function convertToHslCssVar' packages/@core/base/shared/src/color/convert.ts -A 5Length of output: 316
13-17
: Ensure correct HSL conversion.The
convertToHsl
function converts a color to HSL format. Verify that the conversion logic is correct and that the resulting string is in the expected format.Verification successful
Verified: Correct HSL conversion.
The
convertToHsl
function correctly converts a color to HSL format and returns the string in the expected format.
- The function uses the
Color
class to convert the input color to its HSL components.- The HSL values are rounded and formatted into a string.
- If the alpha value is less than 1, it is appended to the HSL string.
The resulting string format is
hsl(h s% l%)
orhsl(h s% l%) a
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `convertToHsl` function logic. # Test: Search for the `convertToHsl` function implementation. Expect: Correct conversion logic. ast-grep --lang typescript --pattern $'function convertToHsl($_) { $$$ }'Length of output: 76
Script:
#!/bin/bash # Search for the `convertToHsl` function definition across the codebase. rg 'function convertToHsl'Length of output: 224
Script:
#!/bin/bash # Extract the implementation of the `convertToHsl` function from the specified file. ast-grep --lang typescript --pattern $'function convertToHsl($_) { $$$ }' packages/@core/base/shared/src/color/convert.tsLength of output: 124
Script:
#!/bin/bash # Search for the `convertToHsl` function in the specified file and include surrounding lines to capture the full implementation. rg -A 10 'function convertToHsl' packages/@core/base/shared/src/color/convert.tsLength of output: 783
34-36
: Ensure correct RGB conversion.The
convertToRgb
function converts a color to RGB format. Verify that the conversion logic is correct and that the resulting string is in the expected format.packages/@core/ui-kit/shadcn-ui/src/components/menu-badge/menu-badge.vue (3)
6-6
: Approved: Import statement forconvertToRgb
.The import statement for
convertToRgb
aligns with the shift from color validation to color conversion.
39-39
: Approved: Return statement inbadgeStyle
computed property.The return statement now ensures that
backgroundColor
is always set to a valid RGB value, improving the reliability of the color application.
37-39
: Approved: Direct conversion to RGB format.The change to use
convertToRgb
for direct conversion to RGB format simplifies the logic and aligns with the new library's capabilities.However, ensure that
badgeClass.value
always contains a valid color value to avoid any potential issues.packages/@core/base/shared/src/color/convert.test.ts (3)
3-3
: Approved: Import statement forconvertToRgb
.The import statement for
convertToRgb
aligns with the shift in focus towards testing RGB conversions.
30-34
: Approved: New test case for converting HSL to RGB format.The new test case validates the conversion of HSL colors to RGB format, ensuring the correctness of the
convertToRgb
function.
36-39
: Approved: New test case for converting HSL with alpha transparency to RGBA format.The new test case validates the conversion of HSL colors with alpha transparency to RGBA format, ensuring the correctness of the
convertToRgb
function.
Description
FastColor 支持的带deg的hsl
Unpacked Size
@ctrl/tinycolor 244 kB
@ant-design/fast-color 43.9 kB
isValidColor移除
FastColor("not-a-color") ->
#000000
所以通过color转换的一定是合法的颜色值
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
Summary by CodeRabbit
New Features
convertToHsl
,convertToHslCssVar
, andconvertToRgb
, enhancing the color processing capabilities.Improvements
TinyColor
library withFastColor
, streamlining color conversions and improving performance.Bug Fixes
Refactor