Skip to content

Conversation

quarksb
Copy link

@quarksb quarksb commented Sep 29, 2025

  1. fix: color of canvas initial error
  2. refactor: rm sass due to sass error(Deprecation Warning [legacy-js-api]: The legacy JS API is deprecated and will be removed in Dart Sass 2.0.0.)
  3. refactor: optimize rgbToHsl function for improved readability and performance

@Copilot Copilot AI review requested due to automatic review settings September 29, 2025 06:13
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses multiple issues including canvas color initialization problems, removes Sass dependencies due to deprecation warnings, and optimizes the rgbToHsl function. The changes modernize the codebase by migrating from Sass to vanilla CSS and improving color coordinate calculations.

  • Fixed canvas color initialization errors with proper pointer positioning and resize handling
  • Removed Sass dependency and converted SCSS syntax to vanilla CSS
  • Optimized rgbToHsl function using Math.max/min and cleaner logic flow

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/core/helper/converters.ts Optimized rgbToHsl function with cleaner algorithm and better performance
src/components/Vue3ColorPicker.vue Major refactor: added pointer positioning logic, lifecycle management, and converted SCSS to CSS
src/components/PickerWrap.vue Removed empty SCSS style block
src/components/PickerMenu.vue Removed empty SCSS style block
src/components/PickerHue.vue Removed empty SCSS style block
src/components/OpacityBar.vue Removed empty SCSS style block
src/components/GradientBar.vue Removed empty SCSS style block
src/components/ColorInputMenu.vue Removed empty SCSS style block
package.json Removed sass dependency and updated TypeScript version

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +231 to +240
switch (max) {
case nr:
h = (ng - nb) / delta;
break;
case ng:
h = 2 + (nb - nr) / delta;
break;
default:
h = 4 + (nr - ng) / delta;
break;
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

The switch statement comparing floating-point numbers with strict equality may fail due to precision issues. Use a tolerance-based comparison or restructure the logic to avoid direct floating-point equality checks.

Suggested change
switch (max) {
case nr:
h = (ng - nb) / delta;
break;
case ng:
h = 2 + (nb - nr) / delta;
break;
default:
h = 4 + (nr - ng) / delta;
break;
const EPSILON = 1e-10;
if (Math.abs(max - nr) < EPSILON) {
h = (ng - nb) / delta;
} else if (Math.abs(max - ng) < EPSILON) {
h = 2 + (nb - nr) / delta;
} else {
h = 4 + (nr - ng) / delta;

Copilot uses AI. Check for mistakes.

pickerWrap.value.offsetHeight - pickerPointer.value.offsetHeight;
if (rightLine <= 0 || bottomLine <= 0) {
if (pointerRetryCount < 5 && pointerUpdateFrame === null) {
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

The magic number 5 for retry count should be extracted as a named constant to improve maintainability and make the retry limit configurable.

Copilot uses AI. Check for mistakes.

Co-authored-by: Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant