Refractored ParseValue and Simplified cycleColorSpace Logic #29840
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
| const [r, g, b, a] = stringToArgs(value); | ||
| const [h, s, l] = convert.rgb.hsl([r, g, b]) || [0, 0, 0]; |
There was a problem hiding this comment.
logic: potential undefined access when convert.rgb.hsl returns null - should handle this case explicitly
| const [h, s, l, a] = stringToArgs(value); | ||
| const [r, g, b] = convert.hsl.rgb([h, s, l]) || [0, 0, 0]; |
There was a problem hiding this comment.
logic: potential undefined access when convert.hsl.rgb returns null - should handle this case explicitly
| const rgb = convert.keyword.rgb(plain as any) || convert.hex.rgb(plain); | ||
| const hsl = convert.rgb.hsl(rgb); |
There was a problem hiding this comment.
style: casting to 'any' here bypasses type safety - consider using proper type checking
| const updatedValue = color?.[nextSpace] || ''; | ||
| setValue(updatedValue); | ||
| onChange(updatedValue); |
There was a problem hiding this comment.
style: setValue and onChange could be batched together to avoid multiple rerenders
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 05994ca. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
Closes #
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
- The color picker still works as expected.
- Changing color formats (RGB/HSL/HEX) cycles correctly.
- Editing the color input updates the component properly.
- Using presets and adding new presets is still functional.
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
Refactored color parsing logic in Color.tsx by breaking down the monolithic parseValue function and simplifying cycleColorSpace calculation for improved maintainability.
parseValueinto three specialized helper functions:parseRgb,parseHsl, andparseHexOrKeywordincode/lib/blocks/src/controls/Color.tsxcycleColorSpacefunction using modulo operator for cleaner index calculation💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!