-
Notifications
You must be signed in to change notification settings - Fork 312
feat(color-picker): refactoring the ColorPicker component style #3595
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
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces significant changes to the color picker and color select panel components, including new two-way data binding with Changes
Sequence Diagram(s)sequenceDiagram
participant ParentComponent
participant TinyColorPicker
participant ColorSelectPanel
ParentComponent->>TinyColorPicker: Bind color via v-model
TinyColorPicker->>ColorSelectPanel: Pass color as prop
ColorSelectPanel->>ColorSelectPanel: Parse color (parseCustomRGBA)
ColorSelectPanel-->>TinyColorPicker: Emit confirm/cancel events
TinyColorPicker-->>ParentComponent: Update color (via v-model)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/renderless/src/color-select-panel/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/renderless/src/color-select-panel/vue.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/vue-locale/src/lang/zh-CN.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
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: 9
🔭 Outside diff range comments (1)
packages/renderless/src/color-select-panel/hue-select/index.ts (1)
43-52
: Function name is misleading and should be renamed.The function is still named
getThumbTop
but now calculates the horizontal position (left
) instead of the vertical position (top
). This creates confusion and reduces code maintainability.- const getThumbTop = () => { + const getThumbLeft = () => {Also update the function call on line 54:
- state.thumbLeft = getThumbTop() + state.thumbLeft = getThumbLeft()
🧹 Nitpick comments (2)
packages/renderless/src/color-select-panel/index.ts (1)
167-172
: Consider using a more descriptive naming convention for the new reactive references.The naming pattern
hexInput1
,hexInput2
,hexInput4-7
is confusing since these are used for non-hex formats as well. Consider using more descriptive names likecolorComponent1
,colorComponent2
, etc.- const hexInput1 = ref<number | string | any>() - const hexInput2 = ref<number | string | any>() - const hexInput4 = ref<number | string | any>() - const hexInput5 = ref<number | string | any>() - const hexInput6 = ref<number | string | any>() - const hexInput7 = ref<number | string | any>() + const colorComponent1 = ref<number | string>() + const colorComponent2 = ref<number | string>() + const colorComponent3 = ref<number | string>() + const colorComponent4 = ref<number | string>() + const colorComponent5 = ref<number | string>() + const colorComponent6 = ref<number | string>()packages/vue/src/color-select-panel/src/components/alpha-select.vue (1)
19-24
: Fix class naming inconsistency.The class name uses "hue-select" but this is the alpha-select component. Consider renaming for consistency.
- class="tiny-color-select-panel__inner__hue-select-thumb-heart" + class="tiny-color-select-panel__inner__alpha-select-thumb-heart"The dynamic background binding to
color.value
is correct and provides good visual feedback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
examples/sites/demos/pc/app/color-picker/alpha-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/color-picker/alpha.vue
(1 hunks)examples/sites/demos/pc/app/color-picker/event-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/color-picker/event.vue
(1 hunks)packages/renderless/src/color-select-panel/hue-select/index.ts
(3 hunks)packages/renderless/src/color-select-panel/index.ts
(4 hunks)packages/renderless/src/color-select-panel/utils/color.ts
(1 hunks)packages/renderless/src/color-select-panel/vue.ts
(2 hunks)packages/theme-saas/src/color-picker/index.less
(1 hunks)packages/theme-saas/src/color-select-panel/index.less
(1 hunks)packages/theme/src/color-picker/index.less
(2 hunks)packages/theme/src/color-picker/vars.less
(1 hunks)packages/theme/src/color-select-panel/index.less
(3 hunks)packages/theme/src/color-select-panel/vars.less
(1 hunks)packages/vue/src/color-picker/src/pc.vue
(1 hunks)packages/vue/src/color-select-panel/src/components/alpha-select.vue
(1 hunks)packages/vue/src/color-select-panel/src/components/hue-select.vue
(1 hunks)packages/vue/src/color-select-panel/src/pc.vue
(2 hunks)
🧰 Additional context used
🧠 Learnings (9)
packages/vue/src/color-select-panel/src/components/hue-select.vue (3)
Learnt from: Davont
PR: opentiny/tiny-vue#2513
File: packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue:30-32
Timestamp: 2024-11-25T03:24:05.740Z
Learning: 在位于`packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue`的组件中,当使用`chart-core`时,应删除错误的`option`定义,使用`chart-core`中的`huiChartOption`。
Learnt from: Davont
PR: opentiny/tiny-vue#2513
File: packages/vue/src/huicharts/huicharts-histogram/src/chart-histogram.vue:33-36
Timestamp: 2024-11-25T03:43:05.285Z
Learning: 在 Tiny Vue 代码库中,使用 `chart-core` 中的 `huiChartOption` 的组件,不应在其 `data` 中定义 `huiChartOption` 或 `option`,而是应该依赖 `chart-core` 提供的 `huiChartOption`。
Learnt from: Davont
PR: opentiny/tiny-vue#2513
File: packages/vue/src/huicharts/huicharts-heatmap/src/chart-heatmap.vue:38-40
Timestamp: 2024-11-25T03:43:19.322Z
Learning: 在 Vue.js 组件(如 `chart-heatmap.vue`)中,使用来自 `chart-core` 的 `huiChartOption` 来管理图表选项,不要在 `data()` 中声明 `option`。
packages/theme-saas/src/color-picker/index.less (1)
Learnt from: zzcr
PR: opentiny/tiny-vue#2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在 `packages/theme/src/time-range/vars.less` 文件中,应使用 `var(--tv-TimeRange-header-height)` 作为 `--tv-TimeRange-header-line-height` 的值,以保持一致性。
packages/theme-saas/src/color-select-panel/index.less (1)
Learnt from: zzcr
PR: opentiny/tiny-vue#2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在 `packages/theme/src/time-range/vars.less` 文件中,应使用 `var(--tv-TimeRange-header-height)` 作为 `--tv-TimeRange-header-line-height` 的值,以保持一致性。
packages/theme/src/color-select-panel/vars.less (1)
Learnt from: zzcr
PR: opentiny/tiny-vue#2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在 `packages/theme/src/time-range/vars.less` 文件中,应使用 `var(--tv-TimeRange-header-height)` 作为 `--tv-TimeRange-header-line-height` 的值,以保持一致性。
packages/renderless/src/color-select-panel/index.ts (1)
Learnt from: zzcr
PR: opentiny/tiny-vue#2584
File: packages/renderless/src/numeric/index.ts:124-127
Timestamp: 2024-12-02T08:56:05.176Z
Learning: 在`packages/renderless/src/numeric/index.ts`文件中,在访问`props.step.value`之前,应先判断`props.step`是否为对象,以确保容错处理。
packages/vue/src/color-select-panel/src/pc.vue (2)
Learnt from: Davont
PR: opentiny/tiny-vue#2513
File: packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue:30-32
Timestamp: 2024-11-25T03:24:05.740Z
Learning: 在位于`packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue`的组件中,当使用`chart-core`时,应删除错误的`option`定义,使用`chart-core`中的`huiChartOption`。
Learnt from: Davont
PR: opentiny/tiny-vue#2513
File: packages/vue/src/huicharts/huicharts-histogram/src/chart-histogram.vue:33-36
Timestamp: 2024-11-25T03:43:05.285Z
Learning: 在 Tiny Vue 代码库中,使用 `chart-core` 中的 `huiChartOption` 的组件,不应在其 `data` 中定义 `huiChartOption` 或 `option`,而是应该依赖 `chart-core` 提供的 `huiChartOption`。
packages/theme/src/color-select-panel/index.less (1)
Learnt from: zzcr
PR: opentiny/tiny-vue#2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在 `packages/theme/src/time-range/vars.less` 文件中,应使用 `var(--tv-TimeRange-header-height)` 作为 `--tv-TimeRange-header-line-height` 的值,以保持一致性。
packages/theme/src/color-picker/vars.less (1)
Learnt from: zzcr
PR: opentiny/tiny-vue#2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在 `packages/theme/src/time-range/vars.less` 文件中,应使用 `var(--tv-TimeRange-header-height)` 作为 `--tv-TimeRange-header-line-height` 的值,以保持一致性。
packages/theme/src/color-picker/index.less (1)
Learnt from: zzcr
PR: opentiny/tiny-vue#2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在 `packages/theme/src/time-range/vars.less` 文件中,应使用 `var(--tv-TimeRange-header-height)` 作为 `--tv-TimeRange-header-line-height` 的值,以保持一致性。
🧬 Code Graph Analysis (2)
packages/renderless/src/color-select-panel/vue.ts (1)
packages/renderless/src/color-select-panel/index.ts (1)
parseCustomRGBA
(128-150)
packages/renderless/src/color-select-panel/hue-select/index.ts (1)
packages/renderless/src/color-select-panel/utils/getClientXY.ts (1)
getClientXY
(41-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
- GitHub Check: Parse Affected Components
🔇 Additional comments (36)
packages/renderless/src/color-select-panel/utils/color.ts (1)
291-291
: LGTM - Minor formatting improvement.The added blank line improves code readability without affecting functionality.
packages/vue/src/color-picker/src/pc.vue (1)
30-30
: Verify that the reduced min-width doesn't negatively impact usability.The min-width reduction from 420px to 330px makes the component more compact, but ensure that all UI elements within the color select panel remain accessible and properly sized.
examples/sites/demos/pc/app/color-picker/event.vue (1)
2-2
: LGTM - Good addition of two-way data binding.Adding
v-model="color"
enables proper reactive synchronization between the component and parent state, which is a Vue best practice.examples/sites/demos/pc/app/color-picker/alpha.vue (1)
2-2
: LGTM - Consistent two-way binding implementation.The addition of
v-model="color"
is consistent with other demo files and properly enables reactive color state management.examples/sites/demos/pc/app/color-picker/event-composition-api.vue (1)
2-2
: LGTM! Good addition of two-way data binding.Adding
v-model="color"
enables proper reactive synchronization between the color picker and parent component state, which is a standard and expected pattern for Vue components.examples/sites/demos/pc/app/color-picker/alpha-composition-api.vue (1)
2-2
: LGTM! Consistent v-model implementation.The addition of
v-model="color"
is consistent with the other demo files and properly enables two-way data binding for the alpha-enabled color picker.packages/vue/src/color-select-panel/src/components/alpha-select.vue (1)
13-13
: LGTM! Positioning adjustment for better visual alignment.The -2px top offset likely improves the visual centering of the thumb element.
packages/vue/src/color-select-panel/src/components/hue-select.vue (3)
7-7
: LGTM! Added CSS class for enhanced styling.The new
tiny-color-select-panel__inner__hue-select-thumb
class enables better styling control for the thumb element.
9-10
: LGTM! Orientation change from vertical to horizontal.The positioning change from
top
withtranslateY
toleft
withtranslateX
correctly implements the horizontal orientation for the hue selector. This aligns with the PR objective of refactoring the component style.
13-18
: LGTM! Visual feedback enhancement.The nested "heart" element with dynamic background color provides excellent visual feedback by showing the selected hue color within the thumb. The binding to
color.value
is appropriate.packages/theme-saas/src/color-picker/index.less (5)
1-4
: LGTM! Proper styling foundation.The import structure and CSS prefix pattern follow established conventions for the theme system.
5-17
: LGTM! Well-structured base styles.The base color-picker styles are comprehensive, including:
- Proper positioning and sizing
- Appropriate cursor and border styling
- Good hover state feedback
- Reasonable default padding
19-37
: LGTM! Comprehensive size variants.The size variants (large, medium, small, mini) provide good flexibility and use consistent Tailwind utility classes.
51-66
: LGTM! Consistent border radius handling.The border radius styling per size variant is well-implemented, using both utility classes and explicit values where appropriate.
39-49
: Confirm hidden SVG rule in color-picker inner containerI searched the
packages/vue/src/color-picker
components and found no inline<svg>
elements, so the following rule in
packages/theme-saas/src/color-picker/index.less
(lines 39–49) currently has no effect:&__inner { @apply flex; @apply w-full; @apply h-full; @apply items-center; @apply justify-center; svg { @apply hidden; } }
• If you plan to render SVG icons (e.g. via slots or custom markers), hiding them here may be intentional.
• Otherwise, consider removing or adjusting this rule to avoid hiding future icons by accident.Please confirm whether this SVG-hiding rule is desired.
packages/renderless/src/color-select-panel/vue.ts (1)
2-2
: LGTM - Import addition is correctThe
parseCustomRGBA
import is properly added and aligns with the new color parsing functionality.packages/theme/src/color-select-panel/vars.less (3)
9-11
: LGTM - New spacing variables are well-definedThe new spacing variables
--tv-ColorSelectPanel-spacing
and--tv-ColorSelectPanel-predefine-spacing
are properly defined and will provide better layout control.
19-19
: LGTM - Box shadow enhancement is appropriateThe increased vertical offset from 2px to 4px enhances the shadow depth, which complements the increased spacing and padding changes.
13-17
: Spacing consistency confirmed in ColorSelectPanelThe updates in packages/theme/src/color-select-panel/vars.less (use of var(--tv-space-xl, 16px) for padding and alpha margin) match the pattern found across other components (e.g., Tree, Tooltip, SelectDropdown, etc.). No further adjustments are necessary.
packages/theme-saas/src/color-select-panel/index.less (3)
1-17
: LGTM - Well-structured root stylesThe root styles are well-organized with proper flex layout, positioning, and shadow styling. The use of utility classes with
@apply
is consistent.
183-204
: LGTM - Alpha selector styling is consistentThe alpha selector styling follows the same pattern as the hue selector with proper thumb and gradient styling.
140-167
: Horizontal hue selector confirmed
Verified that thehue-select
component inpackages/renderless/src/color-select-panel/hue-select/index.ts
implements only a horizontal track (no vertical logic or props), so the CSS gradient and thumb styling inpackages/theme-saas/src/color-select-panel/index.less
correctly align with the JavaScript behavior.packages/vue/src/color-select-panel/src/pc.vue (2)
5-11
: LGTM - Color display enhancementThe addition of the color display div with dynamic background styling improves the user experience by showing the current color selection.
65-70
: LGTM - Button layout improvementThe button reordering and styling (primary confirm, text cancel) follows good UX practices with proper visual hierarchy.
packages/theme/src/color-picker/vars.less (3)
7-7
: LGTM - Border radius update is consistentThe change from
--tv-border-radius-xs
to--tv-border-radius-sm
(2px to 4px) aligns with the overall design system improvements.
14-31
: LGTM - Size variable updates are systematicThe size variable updates show a consistent pattern of reducing component dimensions, which aligns with the overall refactoring objectives. The variable references are appropriate for each size tier.
9-9
: Confirm ColorPicker padding change aligns with design specThe only occurrence of vertical padding in the ColorPicker theme is here, and it’s correctly consumed in
index.less
:• packages/theme/src/color-picker/vars.less:
--tv-ColorPicker-padding-y: 3px;
• packages/theme/src/color-picker/index.less:
padding: var(--tv-ColorPicker-padding-y) var(--tv-ColorPicker-padding-x);
No references to the old
var(--tv-space-xs, 4px)
remain in this component. Please verify that the new fixed value of 3px matches the intended vertical spacing in the design system.packages/renderless/src/color-select-panel/hue-select/index.ts (2)
21-22
: State property correctly updated for horizontal orientation.The change from
thumbTop
tothumbLeft
properly reflects the new horizontal orientation of the hue selector.
65-72
: Drag calculation logic correctly updated for horizontal orientation.The drag event handling properly uses
clientX
instead ofclientY
and calculates the horizontal position with appropriate boundary constraints. The hue value calculation correctly maps the horizontal position to the 0-360 hue range.packages/theme/src/color-picker/index.less (3)
42-52
: Inner element structure properly implemented.The new flex layout for the inner element with centering is well-structured and will provide better content alignment.
54-68
: Size-specific border-radius overrides are well-organized.The progressive border-radius values (4px → 3px → 3px → 2px) for different sizes create a consistent visual hierarchy.
10-11
: No regressions detected for color-picker size change
- Updated default size in
packages/theme/src/color-picker/index.less
from 32×32 to 20×20 px.- Search across CSS, Vue components, TypeScript files, demos and tests found no references or expectations of a 32 px default for color-picker.
- Hover styles and new inner-element structure are scoped to the component and do not introduce specificity conflicts.
packages/theme/src/color-select-panel/index.less (4)
37-103
: Tools section layout is well-structured.The flex layout with proper gap spacing and specific width allocations for different input groups creates a good user experience. The button group with
flex-direction: row-reverse
is a nice touch for proper button ordering.
143-168
: Hue selector horizontal styling is well-implemented.The transformation from vertical to horizontal is properly executed with:
- Correct gradient direction (
to right
)- Proper thumb positioning and sizing
- Good use of border-radius for rounded appearance
- Nice addition of the heart element inside the thumb
The implementation aligns well with the logic changes in the renderless layer.
172-205
: Alpha slider styling matches hue selector design.The consistent styling between alpha and hue selectors (same dimensions, border-radius, thumb design) creates a cohesive user experience. The heart element addition maintains visual consistency.
207-233
: History and predefine sections properly enlarged.The size increase from 20x20 to 28x28 pixels and the improved spacing will enhance usability. The border separation between history and predefine sections provides good visual distinction.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes