feat(color-select-panel): linear-gradient #3738
Conversation
- [x] I/O - [ ] alpha - [ ] predefine - [ ] history
WalkthroughAdds linear-gradient support to color-select-panel: new renderless linear-gradient module, color-points utilities and context, gradient parsing and types, refactors hue/SV/alpha to use shared context, Vue components and styles for gradient UI, demos, tests, and package dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant PC as pc.vue
participant RL as Renderless(index.ts)
participant CTX as ColorPanelContext
participant CP as ColorPoints
participant LG as LinearGradient
participant H as Hue
participant SV as SV
participant A as Alpha
User->>PC: open panel (colorMode)
PC->>RL: initState(props, hooks, utils, ext)
RL->>CTX: createContext(ctx)
alt colorMode == linear-gradient
RL->>CP: parse modelValue -> create colorPoints
CP-->>CTX: ctx.colorPoints + ctx.activeColor + ctx.deg
PC->>LG: mount linear-gradient
else
RL->>CTX: set ctx.activeColor from modelValue
end
User->>H: drag/click hue
H->>CTX: set hue on ctx.activeColor
User->>SV: drag SV
SV->>CTX: set sat/value on ctx.activeColor
User->>A: drag alpha
A->>CTX: set alpha on ctx.activeColor
User->>LG: click gradient bar
LG->>CP: getRelativePos -> addPoint()
CP->>CTX: setActivePoint(newPoint)
User->>LG: drag thumb
LG->>CP: onDrag -> update cursorLeft
CP->>CTX: points updated -> toString(points, deg)
CTX-->>RL: watchers react
RL->>PC: emit update:modelValue / colorUpdate
User->>PC: confirm
PC->>RL: submitValue()
RL->>PC: emit update:modelValue + confirm
sequenceDiagram
autonumber
actor User
participant LG as LinearGradient
participant CP as ColorPoints
participant CTX as Context
User->>LG: click bar at X
LG->>CP: getRelativePos(X) -> create ColorPoint(color, pos)
CP->>CTX: addPoint & setActivePoint
User->>LG: drag thumb
LG->>CP: onDrag -> clamp pos -> update cursorLeft
CP->>CTX: update points
CP->>LG: toString(points, deg)
LG->>CTX: update linearGradientValue
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/renderless/src/color-select-panel/utils/color.ts (2)
204-211: Interface compliance: relax compare signature to IColor (current "this" is too narrow)Using
thismakes the method signature stricter thanIColor.compare(color: IColor). Switch toIColorand read via the publicget(...).- compare(color: this) { - return ( - Math.abs(color._hue - this._hue) < 2 && - Math.abs(color._sat - this._sat) < 1 && - Math.abs(color._value - this._value) < 1 && - Math.abs(color._alpha - this._alpha) < 1 - ) - } + compare(color: IColor) { + return ( + Math.abs(color.get('hue') - this._hue) < 2 && + Math.abs(color.get('sat') - this._sat) < 1 && + Math.abs(color.get('value') - this._value) < 1 && + Math.abs(color.get('alpha') - this._alpha) < 1 + ) + }
285-307: onHsl parsing has two bugs (regex flags and "parent" typo) — breaks HS(L/A) inputs
- The regex has literal “gm” in the pattern instead of flags.
if (parent.length >= 3)references an undefined identifier.Fix both.
- onHsl(value: string) { - const parts = value - .replace(/hsla|hsl\(|\)gm/, '') - .split(/\s|,/g) + onHsl(value: string) { + const parts = value + .replace(/hsla?|\(|\)/gi, '') + .split(/\s|,/g) .filter((val) => val) .map((val, idx) => { return idx > 2 ? Number.parseFloat(val) : Number.parseInt(val, 10) }) if (parts.length === 4) { this._alpha = Number.parseFloat(String(parts[3])) * 100 } else { this._alpha = 100 } - if (parent.length >= 3) { + if (parts.length >= 3) { const { h, s, v } = hsl2hsv({ hue: parts[0], sat: parts[1], light: parts[2] }) this.fromHSV({ h, s, v }) } }packages/renderless/src/color-select-panel/hue-select/index.ts (1)
66-75: Fix hue-to-thumb mapping formula (left position)The current formula is not the inverse of onDrag; thumb misaligns.
Apply:
const getThumbTop = () => { if (!thumb.value) { return 0 } - const hue = ctx.activeColor.value.color.get('hue') - if (!bar.value) { - return 0 - } - return (hue * (bar.value.offsetWidth - thumb.value.offsetWidth / 2)) / 360 + const hue = ctx.activeColor.value.color.get('hue') + const barEl = bar.value + const thumbEl = thumb.value + if (!barEl) return 0 + return (hue / 360) * (barEl.offsetWidth - thumbEl.offsetWidth) + thumbEl.offsetWidth / 2 }
🧹 Nitpick comments (16)
packages/renderless/src/color-select-panel/utils/color.ts (1)
179-185: Harden get(...) against unknown props
this[\_${prop}`]` can yield undefined for invalid keys, violating the declared return type. Add a fallback.get(prop: string) { if (prop === 'alpha') { return Math.floor(this._alpha) } - return this[`_${prop}`] + const v = (this as any)[`_${prop}`] + return typeof v === 'number' ? v : 0 }packages/renderless/src/color-select-panel/utils/color-map.ts (1)
1-150: Make the map immutable (and typed) with "as const"Prevents accidental mutation and preserves literal types.
-export default { +export default { 'black': '#000000', 'silver': '#c0c0c0', ... - 'yellowgreen': '#9acd32' -} + 'yellowgreen': '#9acd32' +} as constpackages/renderless/src/color-select-panel/linear-gradient/vue.ts (2)
73-84: Prefer currentTarget for the thumb element
event.targetmay be a nested child. UsecurrentTarget.- const el = event.target as HTMLElement + const el = (event as any).currentTarget as HTMLElement || (event.target as HTMLElement)
98-109: Rename local toString to avoid shadowing and lint error; also recompute on angle changesAddress Biome’s warning and ensure updates when
degchanges.- const toString = () => { + const linearGradientToString = () => { const colors = context.colorPoints.value @@ - return `linear-gradient(${context.deg.value}deg, ${colors})` + return `linear-gradient(${context.deg.value}deg, ${colors})` } watch( context.colorPoints, () => { - context.linearGardientValue.value = toString() + context.linearGardientValue.value = linearGradientToString() }, { deep: true, immediate: true } ) + watch(context.deg, () => { + context.linearGardientValue.value = linearGradientToString() + })Also applies to: 110-116
packages/renderless/types/color-select-panel.type.ts (2)
22-26: Doc correction: HSL sat/light rangeComments say 0–1 but the implementation takes 0–100 (then divides by 100). Align the comment to avoid confusion.
export interface HSLColor { hue: number // 色调 (0-360) - sat: number // 饱和度 (0-1) - light: number // 亮度 (0-1) + sat: number // 饱和度 (0-100) + light: number // 亮度 (0-100) }
109-113: Naming typo: IColorSelectPanelAlphProps → IColorSelectPanelAlphaPropsPublic type name; fix if still unused externally.
-export interface IColorSelectPanelAlphProps<C> { +export interface IColorSelectPanelAlphaProps<C> { color: C }packages/theme/src/color-select-panel/index.less (1)
178-205: Don’t hardcode red; use a themable or neutral fallbackHardcoding background: red will clash with themes and momentarily show a red bar before JS sets the actual gradient.
Use a themed var with a safe fallback, or remove the background to rely on runtime style:
&__linear-gradient { position: relative; width: 85%; height: 12px; border-radius: 12px; - background: red; + background: var(--tv-ColorSelectPanel-linear-gradient-bg, transparent);Consider adding this var to vars.less if you want a design-approved fallback.
packages/vue/src/color-select-panel/src/components/linear-gradient.vue (1)
9-20: Add touch support for mobile interactionsThumbs only bind mousedown; add touchstart for parity.
- @mousedown="(event) => onThumbMouseDown(event, point)" + @mousedown="(event) => onThumbMouseDown(event, point)" + @touchstart.stop="(event) => onThumbMouseDown(event, point)"Optionally add @touchstart to the bar as well, delegating to onClickBar or a dedicated handler.
examples/sites/demos/pc/app/color-select-panel/linear-gradient.vue (1)
27-27: Consider using a more realistic gradient for the demo.The initial gradient value appears to be a placeholder. Consider using a more visually appealing gradient that better demonstrates the feature's capabilities.
- color: 'linear-gradient(120deg, #66ccffff 50%, #f48fa2 100%)', + color: 'linear-gradient(90deg, #667eea 0%, #764ba2 100%)',packages/renderless/src/color-select-panel/alpha-select/index.ts (1)
60-63: Simplify the conditional check.The nested conditional check can be simplified for better readability.
- if (ctx.activeColor && ctx.activeColor.value) { + if (ctx.activeColor?.value) {packages/renderless/src/color-select-panel/utils/color-points.ts (2)
63-65: Address TODO comments or create tracking issues.The TODO comments indicate incomplete functionality for gradient value updates and SV creation. These should be addressed before merging.
Would you like me to help implement the missing functionality for updating the linear gradient value and creating the linear-gradient SV component, or create tracking issues for these tasks?
53-57: Add null check for wrapper element.The function assumes
wrapper.valueis not null on line 54 with the!assertion, but should handle the null case gracefully.const getRelativePos = (wrapper: IColorSelectPanelRef<HTMLElement | undefined>, point: IColorPoint) => { - const wrapperEl = wrapper.value! + const wrapperEl = wrapper.value + if (!wrapperEl) { + return '0' + } const rect = wrapperEl.getBoundingClientRect() return ((point.cursorLeft / rect.width) * 100).toFixed(0) }packages/renderless/src/color-select-panel/sv-select/index.ts (1)
62-66: Avoid non‑null assertion on wrapper; add null guardPrevents runtime errors if the ref isn’t ready.
Apply:
- return (event: MouseEvent | TouchEvent) => { - const el = wrapper.value! - const rect = el.getBoundingClientRect() + return (event: MouseEvent | TouchEvent) => { + const el = wrapper.value + if (!el) return + const rect = el.getBoundingClientRect()packages/renderless/src/color-select-panel/hue-select/index.ts (1)
39-46: Use ctx for hueValue to keep sources consistentCurrent code mixes props.color with ctx.activeColor; align with ctx.
Apply:
- const hueValue = computed(() => props.color.get('hue')) + const hueValue = computed(() => ctx.activeColor.value.color.get('hue'))packages/renderless/src/color-select-panel/index.ts (2)
337-350: Guarded gradient init is fine; but consider renaming isGrandient → isGradientTypo in helper name; readable API matters. Update usages accordingly.
Apply:
- if (isGrandient(props.modelValue)) { + if (isGradient(props.modelValue)) {And rename definition and other call sites in this file:
-const isGrandient = (val: unknown) => { +const isGradient = (val: unknown) => {- if (!isGrandient(val)) { + if (!isGradient(val)) {
192-197: Nit: function name typo (isGrandient)Spelling nit; see separate comment for optional rename.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
examples/sites/demos/pc/app/color-select-panel/linear-gradient.vue(1 hunks)examples/sites/demos/pc/app/color-select-panel/webdoc/color-select-panel.js(1 hunks)packages/renderless/src/color-select-panel/alpha-select/index.ts(5 hunks)packages/renderless/src/color-select-panel/alpha-select/vue.ts(3 hunks)packages/renderless/src/color-select-panel/hue-select/index.ts(3 hunks)packages/renderless/src/color-select-panel/hue-select/vue.ts(2 hunks)packages/renderless/src/color-select-panel/index.ts(7 hunks)packages/renderless/src/color-select-panel/linear-gradient/vue.ts(1 hunks)packages/renderless/src/color-select-panel/sv-select/index.ts(3 hunks)packages/renderless/src/color-select-panel/sv-select/vue.ts(1 hunks)packages/renderless/src/color-select-panel/utils/color-map.ts(1 hunks)packages/renderless/src/color-select-panel/utils/color-points.ts(1 hunks)packages/renderless/src/color-select-panel/utils/color.ts(2 hunks)packages/renderless/src/color-select-panel/utils/context.ts(1 hunks)packages/renderless/src/color-select-panel/vue.ts(3 hunks)packages/renderless/types/color-select-panel.type.ts(3 hunks)packages/theme/src/color-select-panel/index.less(2 hunks)packages/theme/src/dark-theme-index.less(1 hunks)packages/vue/src/color-select-panel/package.json(1 hunks)packages/vue/src/color-select-panel/src/components/hue-select.vue(2 hunks)packages/vue/src/color-select-panel/src/components/linear-gradient.vue(1 hunks)packages/vue/src/color-select-panel/src/pc.vue(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
packages/renderless/src/color-select-panel/utils/color.ts (1)
packages/renderless/types/color-select-panel.type.ts (1)
IColor(29-69)
packages/renderless/src/color-select-panel/alpha-select/vue.ts (2)
packages/renderless/src/color-select-panel/utils/context.ts (1)
useContext(13-15)packages/renderless/src/color-select-panel/alpha-select/index.ts (3)
initState(12-17)useEvent(19-71)initWatch(73-89)
packages/renderless/src/color-select-panel/hue-select/vue.ts (3)
packages/renderless/src/color-select-panel/hue-select/index.ts (4)
initDom(48-54)initState(39-46)useEvent(56-111)useOnClickBar(20-37)packages/renderless/src/color-select-panel/utils/context.ts (1)
useContext(13-15)packages/renderless/src/color-select-panel/utils/color-points.ts (1)
useColorPoints(17-92)
packages/renderless/src/color-select-panel/utils/context.ts (1)
packages/renderless/types/color-select-panel.type.ts (2)
IColorSelectPanelMaybeRef(93-93)ColorPanelContext(85-92)
packages/renderless/src/color-select-panel/vue.ts (2)
packages/renderless/types/color-select-panel.type.ts (1)
ColorSelectPanelExtends(135-137)packages/renderless/src/color-select-panel/index.ts (2)
initState(307-396)initApi(50-158)
packages/renderless/src/color-select-panel/alpha-select/index.ts (1)
packages/renderless/src/color-select-panel/utils/color.ts (1)
Color(157-418)
packages/renderless/src/color-select-panel/linear-gradient/vue.ts (5)
packages/renderless/src/color-select-panel/utils/context.ts (1)
useContext(13-15)packages/renderless/src/color-select-panel/utils/color-points.ts (1)
ColorPoint(10-15)packages/renderless/src/color-select-panel/utils/getClientXY.ts (1)
getClientXY(41-58)packages/renderless/src/color-select-panel/utils/color.ts (1)
Color(157-418)packages/renderless/types/color-select-panel.type.ts (1)
IColorPoint(80-83)
packages/renderless/src/color-select-panel/hue-select/index.ts (7)
packages/renderless/types/color-select-panel.type.ts (3)
UseColorPointsRet(125-133)IColorSelectPanelHueProps(117-119)ColorPanelContext(85-92)packages/renderless/src/color-select-panel/utils/color.ts (1)
Color(157-418)packages/renderless/src/color-select-panel/utils/color-points.ts (1)
ColorPoint(10-15)packages/renderless/src/color-select-panel/index.ts (1)
initState(307-396)packages/renderless/src/color-select-panel/sv-select/index.ts (1)
initState(14-32)packages/renderless/src/color-select-panel/utils/context.ts (1)
useContext(13-15)packages/renderless/src/color-select-panel/utils/getClientXY.ts (1)
getClientXY(41-58)
packages/renderless/src/color-select-panel/sv-select/vue.ts (2)
packages/renderless/src/color-select-panel/utils/context.ts (1)
useContext(13-15)packages/renderless/src/color-select-panel/sv-select/index.ts (3)
initState(14-32)useUpdate(34-53)useDrag(55-87)
packages/renderless/types/color-select-panel.type.ts (2)
packages/renderless/src/color-select-panel/utils/color.ts (1)
ColorOptions(151-155)packages/renderless/src/color-picker/utils/color.ts (1)
ColorOptions(149-153)
packages/renderless/src/color-select-panel/utils/color-points.ts (2)
packages/renderless/types/color-select-panel.type.ts (4)
IColorPoint(80-83)IColorSelectPanelRef(94-96)IColor(29-69)ColorPanelContext(85-92)packages/renderless/src/color-select-panel/utils/getClientXY.ts (1)
getClientXY(41-58)
packages/renderless/src/color-select-panel/sv-select/index.ts (4)
packages/renderless/src/color-select-panel/index.ts (1)
initState(307-396)packages/renderless/types/color-select-panel.type.ts (3)
IColorSelectPanelSVProps(113-115)IColorSelectPanelRef(94-96)ColorPanelContext(85-92)packages/renderless/src/color-select-panel/utils/color.ts (1)
Color(157-418)packages/renderless/src/color-select-panel/utils/context.ts (1)
useContext(13-15)
packages/renderless/src/color-select-panel/index.ts (4)
packages/renderless/src/color-picker/index.ts (1)
updateModelValue(4-6)packages/renderless/types/color-select-panel.type.ts (16)
ColorSelectPanelExtends(135-137)DirectionalNode(165-180)AngularNode(182-185)HexNode(193-197)RgbNode(199-203)RgbaNode(205-209)HslNode(211-215)HslaNode(217-221)LiteralNode(187-191)CalcNode(275-278)PercentNode(270-273)EmNode(265-268)PxNode(260-263)ColorStop(280-280)IColorSelectPanelProps(97-107)ColorPanelContext(85-92)packages/renderless/src/color-select-panel/utils/color-points.ts (1)
ColorPoint(10-15)packages/renderless/src/color-select-panel/utils/context.ts (1)
createContext(5-11)
🪛 Biome (2.1.2)
packages/renderless/src/color-select-panel/linear-gradient/vue.ts
[error] 98-98: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
packages/renderless/src/color-select-panel/utils/color-points.ts
[error] 62-62: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ 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). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (20)
packages/theme/src/dark-theme-index.less (1)
1-1: LGTM — confirm gradient tokens/mixins are covered by dark themedark-theme-index.less imports only ./base/dark-theme.less, but gradient mixins/vars are defined in packages/theme/src/mixins/common.less and component vars (e.g. packages/theme/src/tag/vars.less, packages/theme/src/skeleton-item/vars.less, packages/theme/src/grid/, packages/theme/src/color-select-panel/index.less); base/dark-theme.less has no gradient definitions — add dark variants or import those files into the dark-theme index so gradient styles are included.
examples/sites/demos/pc/app/color-select-panel/webdoc/color-select-panel.js (1)
19-30: Demo entry addition — LGTM (file exists)Confirmed examples/sites/demos/pc/app/color-select-panel/linear-gradient.vue is present.
packages/renderless/src/color-select-panel/alpha-select/vue.ts (1)
19-22: LGTM: ctx integration into event/watch flowPassing ctx into useEvent/initWatch aligns with the new context model.
Please confirm that a provider (createContext) is established in the parent so useContext(hooks) resolves properly at runtime.
Also applies to: 41-41
packages/vue/src/color-select-panel/src/pc.vue (1)
85-112: Parse injection looks correctImporting parse and passing it via extendOptions is a clean way to wire the parser without polluting component API.
packages/vue/src/color-select-panel/src/components/hue-select.vue (1)
16-22: Context-driven hue UI and conditional linear-gradient block look good
- Using state.ctx.activeColor.color.value for the thumb is consistent with the shared context.
- Conditional render of tied to state.ctx.colorMode is appropriate.
Also applies to: 29-42
packages/renderless/src/color-select-panel/sv-select/vue.ts (1)
18-27: Threading ctx OK — initWatch signature and context provider confirmedinitWatch in packages/renderless/src/color-select-panel/sv-select/index.ts is (state, update, { watch }) — keep initWatch(state, update, hooks). createContext(ctx, hooks) is present in packages/renderless/src/color-select-panel/index.ts, so useContext(hooks) will be provided.
packages/renderless/src/color-select-panel/vue.ts (3)
1-6: LGTM! Clean type import additions.The import statement correctly adds
ColorSelectPanelExtendsalongside existing types, maintaining proper organization.
32-32: LGTM! Consistent parameter propagation.The
extandutilsparameters are correctly passed toinitState, andhooksis appropriately added toinitApi. This ensures proper context sharing across initialization functions.Also applies to: 47-47
26-31: Confirm callers updated to passext(has parse)?I inspected the repo: packages/renderless/src/color-select-panel/vue.ts now accepts
ext: ColorSelectPanelExtends. The only component importing this renderless is packages/vue/src/color-select-panel/src/pc.vue — open that file and confirm it calls renderless with ansdk/extensions object that contains aparseimplementation; otherwise callers must be updated to pass one. I couldn't find other callers of this specific renderless export.packages/renderless/src/color-select-panel/hue-select/vue.ts (2)
4-6: LGTM! Proper context integration.The imports are correctly added to support context-based color management and color points functionality.
58-64: LGTM! Proper reactive synchronization.The watch implementation correctly synchronizes
thumbLeftwith the active color's hue value through the context.packages/renderless/src/color-select-panel/alpha-select/index.ts (2)
1-6: LGTM! Type imports properly updated.The addition of
ColorPanelContextto the imports is correct and follows the established pattern.
40-40: LGTM! Consistent context-based color management.The replacement of
props.colorwithctx.activeColor.value.coloris implemented consistently throughout the module, maintaining proper separation of concerns.Also applies to: 56-56
packages/renderless/src/color-select-panel/utils/color-points.ts (1)
10-15: LGTM! Clean ColorPoint implementation.The
ColorPointclass is properly implemented with public properties following TypeScript best practices.packages/renderless/src/color-select-panel/index.ts (6)
198-205: Directional degree map: looks goodCovers all CSS named corners/edges.
34-49: Event API helpers look goodClear separation of updateModelValue/triggerConfirm/Cancel/ColorUpdate.
50-58: initApi signature change is reasonableSeparating hooks/utils is consistent with other modules.
398-405: Color update watch emits object; verify downstream expectationsIf consumers expect string, adjust. Otherwise OK.
Would you confirm listeners of 'color-update' expect a Color instance, not a string?
329-335: Context initialization LGTMContext fields and defaults match types.
56-58: Public API wiring looks consistentopen/close/reset/on* handlers are coherent with previous design.
Also applies to: 144-158
examples/sites/demos/pc/app/color-select-panel/linear-gradient.vue
Outdated
Show resolved
Hide resolved
GaoNeng-wWw
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/renderless/src/color-select-panel/utils/color.ts (1)
285-299: Fix HSL parsing regex (currently broken flags placement)The regex places “gm” inside the pattern, so it won’t strip tokens correctly. Use flags properly.
- const parts = value - .replace(/hsla|hsl\(|\)gm/, '') + const parts = value + .replace(/hsla|hsl|\(|\)/gm, '') .split(/\s|,/g)packages/renderless/src/color-select-panel/alpha-select/index.ts (1)
30-44: Guard alphaWrapper before using getBoundingClientRectalphaWrapper is not checked; calling getBoundingClientRect on null will throw.
const onDrag = (event: MouseEvent | TouchEvent) => { if (!slider.value || !alphaThumb.value) { return } - const el = alphaWrapper.value! + const el = alphaWrapper.value + if (!el) { + return + } const rect = el.getBoundingClientRect()packages/renderless/src/color-select-panel/index.ts (2)
469-485: Fix watcher to use ref values (and parse with correct args)The watcher operates on Ref objects; use .value for correctness.
- watch( - () => [state.currentColor, state.linearGradient], + watch( + () => [state.currentColor.value, state.linearGradient.value], () => { - if (state.isLinearGradient) { - state.input = state.linearGradient + if (state.isLinearGradient.value) { + state.input = state.linearGradient.value return } else { - state.input = state.currentColor - const result = parseCustomRGBA(state.currentColor, state.currentFormat) + state.input = state.currentColor.value + const result = parseCustomRGBA(state.currentColor.value, state.currentFormat.value) state.hexInput4 = Math.ceil(Number(result[0])) state.hexInput5 = result[1] state.hexInput6 = result[2] state.hexInput7 = `${(Number(result[3]) || 1) * 100}%` } triggerColorUpdate(state.input, emit) },
340-426: Fix incorrect use of ctx refs (missing .value) — breaks comparisons/assignmentsMultiple places use state.ctx. (or ctx.) directly while those props are refs/computed; change reads/writes to use .value and assign to .value when updating.
- packages/renderless/src/color-select-panel/index.ts:225–227 — if (state.ctx.colorMode === 'linear-gradient') → state.ctx.colorMode.value === 'linear-gradient'; pass state.ctx.linearGardientValue.value to updateModelValue/triggerConfirm.
- packages/renderless/src/color-select-panel/index.ts:292–306 — use state.ctx.colorMode.value, state.ctx.activeColor.value.color.fromString(historyColor); assign state.ctx.colorPoints.value = colorPoints.colorPoints, state.ctx.activeColor.value = lastPoint, state.ctx.deg.value = colorPoints.angular.
- packages/renderless/src/color-select-panel/index.ts:308–322 — same fixes for onPredefineColorClick (use .value for colorMode, colorPoints, activeColor, deg).
Search for other state.ctx.* usages and update any remaining missing .value accesses.
🧹 Nitpick comments (7)
packages/theme/src/color-select-panel/index.less (2)
48-52: Use flex-end and apply the theme variable for deg text size“justify-content: end” may not be consistently supported; prefer “flex-end”. Also consume the new font-size var so the knob respects theming.
&-deg { display: flex; - justify-content: end; + justify-content: flex-end; + font-size: var(--tv-ColorSelectPanel-deg-text-size); }
183-209: Deduplicate thumb styles via a shared mixinThe thumb block duplicates the hue/alpha thumb styles. Extract to a LESS mixin to reduce drift.
Example (outside this block):
.thumb-base() { position: absolute; top: -2px; width: 16px; height: 16px; background: #fff; box-shadow: 0 0 8px rgba(0, 0, 0, 0.2); border-radius: 16px; &-heart { position: absolute; top: 50%; left: 50%; width: 12px; height: 12px; border-radius: 50%; transform: translate(-50%, -50%); } }Then:
&__thumb { .thumb-base(); }packages/vue/src/color-select-panel/src/pc.vue (1)
29-31: Constrain deg input to valid angle rangeClamp to 0–360 and step by 1 to avoid out-of-range values via scroll.
- <tiny-numeric v-model="state.ctx.deg" unit="deg" mouse-wheel /> + <tiny-numeric v-model="state.ctx.deg" unit="deg" mouse-wheel :min="0" :max="360" :step="1" />packages/renderless/types/color-select-panel.type.ts (1)
292-294: Rename exported type aliasparse→ParseFnLowercase type alias shadows a value name; update the exported type and the interface property in packages/renderless/types/color-select-panel.type.ts.
Change:
-export type parse = (value: string) => GradientNode[] +export type ParseFn = (value: string) => GradientNode[]And:
-export interface ColorSelectPanelExtends { - parse: parse -} +export interface ColorSelectPanelExtends { + parse: ParseFn +}examples/sites/demos/pc/app/color-select-panel/linear-gradient.spec.ts (2)
3-10: Add at least one outcome assertion in the first testThe first test performs interactions but doesn’t assert the resulting value/state beyond thumb count. Add a final expectation to verify behavior (e.g., input value updated).
test('线性渐变', async ({ page }) => { await page.goto('color-select-panel#linear-gradient') await page.locator('#linear-gradient').getByRole('button', { name: 'Show Color select panel' }).click() expect(await page.locator('.tiny-color-select-panel__linear-gradient__thumb').count()).toBe(2) await page.locator('.tiny-color-select-panel__linear-gradient__thumb').first().click() await page.locator('.tiny-color-select-panel__linear-gradient').click() await page.locator('.tiny-color-select-panel__linear-gradient > div:nth-child(2)').click() + await expect(page.getByLabel('示例', { exact: true }).getByRole('textbox')) + .toContainText('linear-gradient(') })
6-10: Avoid brittle nth-child and deep CSS selectorsThese are likely to flake with structure changes. Prefer role, label, or data-testid.
- await page.locator('.tiny-color-select-panel__linear-gradient > div:nth-child(2)').click() + await page.locator('[data-testid="linear-gradient-bar"]').click() @@ - await page.locator('.tiny-color-select-panel__history__color-block').first().click() + await page.locator('[data-testid="history-color"]').first().click() @@ - await page.locator('.tiny-color-select-panel__history > div:nth-child(2)').click() + await page.locator('[data-testid="history-color"]').nth(1).click()Follow-up: add these data-testid attributes in the demo/component markup.
Also applies to: 28-31, 60-63
packages/renderless/src/color-select-panel/alpha-select/index.ts (1)
22-29: Props parameter is unusedEither remove it from the signature or prefix with underscore to signal intentional unused.
-export const useEvent = ( +export const useEvent = ( state: State, slider: IColorSelectPanelRef<HTMLElement | undefined>, alphaWrapper: IColorSelectPanelRef<HTMLElement | undefined>, alphaThumb: IColorSelectPanelRef<HTMLElement | undefined>, - props: IColorSelectPanelAlphProps<Color>, + _props: IColorSelectPanelAlphProps<Color>, ctx: ColorPanelContext ) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
examples/sites/demos/pc/app/color-select-panel/linear-gradient-composition-api.vue(1 hunks)examples/sites/demos/pc/app/color-select-panel/linear-gradient.spec.ts(1 hunks)examples/sites/demos/pc/app/color-select-panel/linear-gradient.vue(1 hunks)packages/renderless/src/color-select-panel/alpha-select/index.ts(5 hunks)packages/renderless/src/color-select-panel/hue-select/index.ts(4 hunks)packages/renderless/src/color-select-panel/index.ts(8 hunks)packages/renderless/src/color-select-panel/linear-gradient/index.ts(1 hunks)packages/renderless/src/color-select-panel/linear-gradient/vue.ts(1 hunks)packages/renderless/src/color-select-panel/utils/color.ts(3 hunks)packages/renderless/src/color-select-panel/vue.ts(3 hunks)packages/renderless/types/color-select-panel.type.ts(3 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-select-panel/package.json(1 hunks)packages/vue/src/color-select-panel/src/components/alpha-select.vue(1 hunks)packages/vue/src/color-select-panel/src/components/linear-gradient.vue(1 hunks)packages/vue/src/color-select-panel/src/pc.vue(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/vue/src/color-select-panel/src/components/alpha-select.vue
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/vue/src/color-select-panel/package.json
- examples/sites/demos/pc/app/color-select-panel/linear-gradient.vue
- packages/vue/src/color-select-panel/src/components/linear-gradient.vue
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-04T09:35:13.159Z
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` 的值，以保持一致性。
Applied to files:
packages/theme/src/color-select-panel/vars.less
🧬 Code graph analysis (8)
packages/renderless/src/color-select-panel/linear-gradient/index.ts (3)
packages/renderless/types/color-select-panel.type.ts (3)
LinearGradientState(3-5)ColorPanelContext(89-96)IColorPoint(84-87)packages/renderless/src/color-select-panel/utils/color-points.ts (1)
ColorPoint(10-15)packages/renderless/src/color-select-panel/utils/getClientXY.ts (1)
getClientXY(41-58)
packages/renderless/src/color-select-panel/alpha-select/index.ts (5)
packages/renderless/src/color-select-panel/hue-select/index.ts (1)
initState(39-46)packages/renderless/src/color-select-panel/index.ts (1)
initState(340-428)packages/renderless/src/color-select-panel/sv-select/index.ts (1)
initState(14-32)packages/renderless/src/color-select-panel/utils/context.ts (1)
useContext(13-15)packages/renderless/types/color-select-panel.type.ts (2)
IColorSelectPanelAlphProps(113-115)ColorPanelContext(89-96)
packages/renderless/src/color-select-panel/vue.ts (2)
packages/renderless/types/color-select-panel.type.ts (1)
ColorSelectPanelExtends(139-141)packages/renderless/src/color-select-panel/index.ts (2)
initState(340-428)initApi(202-338)
packages/renderless/src/color-select-panel/linear-gradient/vue.ts (2)
packages/renderless/src/color-select-panel/utils/context.ts (1)
useContext(13-15)packages/renderless/src/color-select-panel/linear-gradient/index.ts (2)
initState(138-143)useLinearGradient(17-136)
packages/renderless/src/color-select-panel/index.ts (7)
packages/renderless/types/color-select-panel.type.ts (16)
IColorSelectPanelProps(101-111)ColorSelectPanelExtends(139-141)DirectionalNode(169-184)AngularNode(186-189)HexNode(197-201)RgbNode(203-207)RgbaNode(209-213)HslNode(215-219)HslaNode(221-225)LiteralNode(191-195)CalcNode(279-282)PercentNode(274-277)EmNode(269-272)PxNode(264-267)ColorStop(284-284)ColorPanelContext(89-96)packages/renderless/src/color-select-panel/utils/color-points.ts (1)
ColorPoint(10-15)packages/renderless/src/color-picker/utils/color.ts (1)
Color(155-425)packages/renderless/src/color-select-panel/hue-select/index.ts (1)
initState(39-46)packages/renderless/src/color-select-panel/alpha-select/index.ts (1)
initState(13-20)packages/renderless/src/color-select-panel/sv-select/index.ts (1)
initState(14-32)packages/renderless/src/color-select-panel/utils/context.ts (1)
createContext(5-11)
packages/renderless/src/color-select-panel/utils/color.ts (2)
packages/renderless/src/color-picker/utils/color.ts (1)
Color(155-425)packages/renderless/types/color-select-panel.type.ts (1)
IColor(33-73)
packages/renderless/src/color-select-panel/hue-select/index.ts (6)
packages/renderless/types/color-select-panel.type.ts (3)
UseColorPointsRet(129-137)IColorSelectPanelHueProps(121-123)ColorPanelContext(89-96)packages/renderless/src/color-select-panel/utils/color.ts (1)
Color(157-418)packages/renderless/src/color-select-panel/utils/color-points.ts (1)
ColorPoint(10-15)packages/renderless/src/color-select-panel/index.ts (1)
initState(340-428)packages/renderless/src/color-select-panel/utils/context.ts (1)
useContext(13-15)packages/renderless/src/color-select-panel/utils/getClientXY.ts (1)
getClientXY(41-58)
packages/renderless/types/color-select-panel.type.ts (2)
packages/renderless/src/color-select-panel/utils/color.ts (1)
ColorOptions(151-155)packages/renderless/src/color-picker/utils/color.ts (1)
ColorOptions(149-153)
🪛 Biome (2.1.2)
packages/renderless/src/color-select-panel/linear-gradient/index.ts
[error] 99-99: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ 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). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (16)
examples/sites/demos/pc/app/color-select-panel/linear-gradient-composition-api.vue (1)
24-35: LGTM: Good demo coverage for linear-gradient modeProps and events are wired correctly; example is clear.
packages/renderless/src/color-select-panel/utils/color.ts (1)
1-1: Ensure TS path alias for "@/types" resolves across all buildsIf "@/types" isn’t mapped in all tsconfig variants, builds will break. Either add a paths mapping or switch to a relative import.
#!/bin/bash # Check path alias config and usages rg -nC2 '"baseUrl"|"paths"' --type=json tsconfig*.json rg -nP "from\\s+['\"]@/types['\"]" --type=ts --type=tsx --type=vuepackages/vue/src/color-select-panel/src/pc.vue (2)
9-10: Background binding is now correct for gradient vs solidSwitching between gradient and solid works as expected.
90-90: Verify gradient-parser import interop for packages/vue/src/color-select-panelConfirmed: packages/vue/src/color-select-panel/package.json declares "gradient-parser" and packages/vue/src/color-select-panel/src/pc.vue imports parse as a named export (import { parse } from 'gradient-parser').
Ensure your build supports CJS→ESM named-export interop; if not, switch to a default import and use gp.parse (e.g. import gp from 'gradient-parser'; extendOptions: { parse: gp.parse }).
packages/renderless/types/color-select-panel.type.ts (2)
89-96: Typo in public API: linearGardientValue → linearGradientValueFix before shipping; update all usages to avoid API debt.
export interface ColorPanelContext { colorMode: ComputedRef<IColorSelectPanelProps['colorMode']> activeColor: Ref<IColorPoint> bar: Ref<HTMLElement | null> colorPoints: Ref<IColorPoint[]> - linearGardientValue: Ref<string> + linearGradientValue: Ref<string> deg: Ref<number> }
129-137: Typo in method name: getActviePoint → getActivePointRename and update all call sites.
export interface UseColorPointsRet { onClick: (element: Element, point: IColorPoint) => void linearGradientValue: Readonly<Ref<string>> updateDeg: (_deg: number) => void removePoint: (point: IColorPoint) => void addPoint: (point: IColorPoint) => void setActivePoint: (point: IColorPoint) => void - getActviePoint: () => Ref<IColorPoint> + getActivePoint: () => Ref<IColorPoint> }packages/renderless/src/color-select-panel/vue.ts (1)
26-33: Renderless signature change: confirm all call sites pass extrenderless now expects (props, hooks, utils, ext). Ensure every consumer (pc/mobile/2.x/3.x variants) passes extendOptions with parse to avoid runtime errors.
packages/renderless/src/color-select-panel/linear-gradient/vue.ts (1)
5-20: API surface is consistent — LGTMExposed keys match the returned object. No mismatches detected.
packages/renderless/src/color-select-panel/linear-gradient/index.ts (3)
29-37: Guard ref before getBoundingClientRect (vm.$refs[LINEAR_GRADIENT_BAR])Prevents runtime error when ref isn’t ready.
const getPos = (event: MouseEvent | TouchEvent) => { if (!vm) { return 0 } - const el = vm.$refs[LINEAR_GRADIENT_BAR] as HTMLElement + const el = vm.$refs[LINEAR_GRADIENT_BAR] as HTMLElement | undefined + if (!el) { + return 0 + } const rect = el.getBoundingClientRect()
61-74: Handle THUMB ref as single element or array; also guard vmCurrent code assumes array and will throw if a single ref is returned.
- nextTick(() => { - const lastColorPointElement = (vm.$refs[THUMB] as HTMLElement[]).at(-1) + nextTick(() => { + if (!vm) return + const refEl = vm.$refs[THUMB] as HTMLElement | HTMLElement[] | undefined + const lastColorPointElement = Array.isArray(refEl) ? refEl.at(-1) : refEl if (!lastColorPointElement) { return } draggable(lastColorPointElement, {
118-134: Normalize THUMB refs on mount and set bar ref safelySupports both array and single refs; avoids assigning undefined.
hooks.onMounted(() => { - const elements = vm.$refs[THUMB] as HTMLElement[] - if (!elements || !elements.length) { - return - } - elements.forEach((el) => { + const refEl = vm?.$refs[THUMB] as HTMLElement | HTMLElement[] | undefined + const elements = Array.isArray(refEl) ? refEl : refEl ? [refEl] : [] + elements.forEach((el) => { draggable(el, { drag(event) { onDrag(event) }, end(event) { onDrag(event) } }) }) - context.bar.value = vm.$refs[LINEAR_GRADIENT_BAR] + context.bar.value = (vm?.$refs[LINEAR_GRADIENT_BAR] as HTMLElement | undefined) ?? null })packages/renderless/src/color-select-panel/hue-select/index.ts (1)
20-37: Broaden click handler to support touch; guard bar before useCurrent typing blocks touch and uses non-null assertion on bar.
export const useOnClickBar = ( { addPoint, setActivePoint, getActviePoint }: UseColorPointsRet, { bar }: DomInit, - getLeft: (barEl: HTMLElement, event: MouseEvent) => number + getLeft: (barEl: HTMLElement, event: MouseEvent | TouchEvent) => number ) => { - return (event: MouseEvent) => { + return (event: MouseEvent | TouchEvent) => { const activePoint = getActviePoint().value const color = new Color({ enableAlpha: activePoint.color.enableAlpha, format: activePoint.color.format, value: activePoint.color.value }) - const left = getLeft(bar.value!, event) + const barEl = bar.value + if (!barEl) return + const left = getLeft(barEl, event) const colorPoint = new ColorPoint(color, left) addPoint(colorPoint) setActivePoint(colorPoint) } }packages/renderless/src/color-select-panel/index.ts (4)
225-230: Fix Ref misuse in submitValue (use .value)Comparing/using refs directly breaks logic and emits wrong payloads.
const submitValue = () => { - if (state.ctx.colorMode === 'linear-gradient') { - updateModelValue(state.ctx.linearGardientValue, emit) - triggerConfirm(state.ctx.linearGardientValue, emit) + if (state.ctx.colorMode.value === 'linear-gradient') { + updateModelValue(state.ctx.linearGardientValue.value, emit) + triggerConfirm(state.ctx.linearGardientValue.value, emit) setShowPicker(false) return }
150-153: Fix var() fallback: avoid double ‘#’hex parser expects a value without leading '#'.
'var': (node: any) => { hooks.warn('unsupported var ref.') - return parseBehavior.hex({ type: 'hex', value: '#000', length: node.length }) + return parseBehavior.hex({ type: 'hex', value: '000', length: node.length }) }
164-174: Fix px conversion: don’t divide by widthReturning parseInt(numberValue / width) collapses px to 0/1.
if (node.type === '%') { return Number.parseInt(`${(numberValue / 100) * width}`) } if (node.type === 'px') { - return Number.parseInt(`${numberValue / width}`) + return Math.round(numberValue) }
130-144: RGBA/HSLA parsing: use supported format and preserve alphaUse format 'rgb' (not 'rgba') with enableAlpha=true; keep HSLA already correct.
- rgba: (node: RgbaNode) => { - const color = new Color({ enableAlpha: props.alpha, format: 'rgba', value: `rgba(${node.value.join(',')})` }) - return new ColorPoint(color, 0) - }, + rgba: (node: RgbaNode) => { + const color = new Color({ enableAlpha: props.alpha, format: 'rgb', value: `rgba(${node.value.join(',')})` }) + return new ColorPoint(color, 0) + }, @@ - hsla: (node: HslaNode) => { - const color = new Color({ enableAlpha: props.alpha, format: 'hsl', value: `hsl(${node.value.join(' ')})` }) + hsla: (node: HslaNode) => { + const color = new Color({ enableAlpha: props.alpha, format: 'hsl', value: `hsla(${node.value.join(' ')})` }) return new ColorPoint(color, 0) },
| if (state.ctx.colorMode === 'monochrome') { | ||
| state.ctx.activeColor.color.fromString(historyColor) | ||
| return | ||
| } | ||
| const colorString = isGrandient(historyColor) | ||
| ? historyColor | ||
| : `linear-gradient(90deg, #fff 0%, ${historyColor} 100%)` | ||
| const colorPoints = createColorPoints(colorString, props, hooks, ext, state.ctx.bar) | ||
| state.ctx.colorPoints = colorPoints.colorPoints | ||
| const lastPoint = colorPoints.colorPoints.at(-1) | ||
| if (lastPoint) { | ||
| state.ctx.activeColor = lastPoint | ||
| } | ||
| state.ctx.deg = colorPoints.angular | ||
| } |
There was a problem hiding this comment.
Fix multiple Ref dereferences in onHistoryClick
All ctx fields here are Refs; also pass bar.value to parser.
- if (state.ctx.colorMode === 'monochrome') {
- state.ctx.activeColor.color.fromString(historyColor)
+ if (state.ctx.colorMode.value === 'monochrome') {
+ state.ctx.activeColor.value.color.fromString(historyColor)
return
}
const colorString = isGrandient(historyColor)
? historyColor
: `linear-gradient(90deg, #fff 0%, ${historyColor} 100%)`
- const colorPoints = createColorPoints(colorString, props, hooks, ext, state.ctx.bar)
- state.ctx.colorPoints = colorPoints.colorPoints
+ const colorPoints = createColorPoints(colorString, props, hooks, ext, state.ctx.bar.value)
+ state.ctx.colorPoints.value = colorPoints.colorPoints
const lastPoint = colorPoints.colorPoints.at(-1)
if (lastPoint) {
- state.ctx.activeColor = lastPoint
+ state.ctx.activeColor.value = lastPoint
}
- state.ctx.deg = colorPoints.angular
+ state.ctx.deg.value = colorPoints.angular📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (state.ctx.colorMode === 'monochrome') { | |
| state.ctx.activeColor.color.fromString(historyColor) | |
| return | |
| } | |
| const colorString = isGrandient(historyColor) | |
| ? historyColor | |
| : `linear-gradient(90deg, #fff 0%, ${historyColor} 100%)` | |
| const colorPoints = createColorPoints(colorString, props, hooks, ext, state.ctx.bar) | |
| state.ctx.colorPoints = colorPoints.colorPoints | |
| const lastPoint = colorPoints.colorPoints.at(-1) | |
| if (lastPoint) { | |
| state.ctx.activeColor = lastPoint | |
| } | |
| state.ctx.deg = colorPoints.angular | |
| } | |
| if (state.ctx.colorMode.value === 'monochrome') { | |
| state.ctx.activeColor.value.color.fromString(historyColor) | |
| return | |
| } | |
| const colorString = isGrandient(historyColor) | |
| ? historyColor | |
| : `linear-gradient(90deg, #fff 0%, ${historyColor} 100%)` | |
| const colorPoints = createColorPoints(colorString, props, hooks, ext, state.ctx.bar.value) | |
| state.ctx.colorPoints.value = colorPoints.colorPoints | |
| const lastPoint = colorPoints.colorPoints.at(-1) | |
| if (lastPoint) { | |
| state.ctx.activeColor.value = lastPoint | |
| } | |
| state.ctx.deg.value = colorPoints.angular | |
| } |
🤖 Prompt for AI Agents
In packages/renderless/src/color-select-panel/index.ts around lines 292–306, the
onHistoryClick block is directly dereferencing ctx fields that are Vue Refs;
change accesses to use .value (e.g., state.ctx.colorMode.value,
state.ctx.activeColor.value.color.fromString(...), state.ctx.colorPoints.value =
..., state.ctx.deg.value = ..., and when passing the bar to the parser use
state.ctx.bar.value). Also when assigning the active color use the lastPoint (or
its value) correctly via refs (e.g., state.ctx.activeColor.value = lastPoint),
and ensure createColorPoints receives state.ctx.bar.value instead of
state.ctx.bar.
| if (state.ctx.colorMode === 'monochrome') { | ||
| state.color.fromString(predefineColor) | ||
| return | ||
| } | ||
| const colorString = isGrandient(predefineColor) | ||
| ? predefineColor | ||
| : `linear-gradient(180deg, #fff 0%, ${predefineColor} 100%)` | ||
| const colorPoints = createColorPoints(colorString, props, hooks, ext, state.ctx.bar) | ||
| state.ctx.colorPoints = colorPoints.colorPoints | ||
| const lastPoint = colorPoints.colorPoints.at(-1) | ||
| if (lastPoint) { | ||
| state.ctx.activeColor = lastPoint | ||
| } | ||
| state.ctx.deg = colorPoints.angular | ||
| } |
There was a problem hiding this comment.
Fix multiple Ref dereferences in onPredefineColorClick
Same issues as above.
- if (state.ctx.colorMode === 'monochrome') {
- state.color.fromString(predefineColor)
+ if (state.ctx.colorMode.value === 'monochrome') {
+ state.color.fromString(predefineColor)
return
}
const colorString = isGrandient(predefineColor)
? predefineColor
: `linear-gradient(180deg, #fff 0%, ${predefineColor} 100%)`
- const colorPoints = createColorPoints(colorString, props, hooks, ext, state.ctx.bar)
- state.ctx.colorPoints = colorPoints.colorPoints
+ const colorPoints = createColorPoints(colorString, props, hooks, ext, state.ctx.bar.value)
+ state.ctx.colorPoints.value = colorPoints.colorPoints
const lastPoint = colorPoints.colorPoints.at(-1)
if (lastPoint) {
- state.ctx.activeColor = lastPoint
+ state.ctx.activeColor.value = lastPoint
}
- state.ctx.deg = colorPoints.angular
+ state.ctx.deg.value = colorPoints.angular📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (state.ctx.colorMode === 'monochrome') { | |
| state.color.fromString(predefineColor) | |
| return | |
| } | |
| const colorString = isGrandient(predefineColor) | |
| ? predefineColor | |
| : `linear-gradient(180deg, #fff 0%, ${predefineColor} 100%)` | |
| const colorPoints = createColorPoints(colorString, props, hooks, ext, state.ctx.bar) | |
| state.ctx.colorPoints = colorPoints.colorPoints | |
| const lastPoint = colorPoints.colorPoints.at(-1) | |
| if (lastPoint) { | |
| state.ctx.activeColor = lastPoint | |
| } | |
| state.ctx.deg = colorPoints.angular | |
| } | |
| if (state.ctx.colorMode.value === 'monochrome') { | |
| state.color.fromString(predefineColor) | |
| return | |
| } | |
| const colorString = isGrandient(predefineColor) | |
| ? predefineColor | |
| : `linear-gradient(180deg, #fff 0%, ${predefineColor} 100%)` | |
| const colorPoints = createColorPoints( | |
| colorString, | |
| props, | |
| hooks, | |
| ext, | |
| state.ctx.bar.value | |
| ) | |
| state.ctx.colorPoints.value = colorPoints.colorPoints | |
| const lastPoint = colorPoints.colorPoints.at(-1) | |
| if (lastPoint) { | |
| state.ctx.activeColor.value = lastPoint | |
| } | |
| state.ctx.deg.value = colorPoints.angular | |
| } |
🤖 Prompt for AI Agents
In packages/renderless/src/color-select-panel/index.ts around lines 308 to 322,
the onPredefineColorClick handler repeatedly dereferences reactive refs (e.g.,
state.ctx, state.ctx.colorPoints, props, hooks, ext) causing multiple .value
accesses and potential null/undefined reads; cache required refs/values into
local constants once at the top of the function, perform null checks on those
cached values, then use the locals for branching and assignments (e.g., read
colorMode, predefineColor, and bar into locals, compute colorString and
colorPoints using those locals, set state.ctx.* only once using the cached ctx
reference) to avoid repeated ref dereferences and reduce risk of runtime errors.
| const getRelativePos = (points: IColorPoint) => { | ||
| const bar = vm.$refs[LINEAR_GRADIENT_BAR] as HTMLElement | ||
| if (!bar) { | ||
| return 0 | ||
| } | ||
| const rect = bar.getBoundingClientRect() | ||
| return Number.parseInt(((points.cursorLeft / rect.width) * 100).toFixed(0)) | ||
| } |
There was a problem hiding this comment.
Guard vm before refs in getRelativePos
Accessing vm.$refs when vm is falsy will throw.
const getRelativePos = (points: IColorPoint) => {
- const bar = vm.$refs[LINEAR_GRADIENT_BAR] as HTMLElement
- if (!bar) {
+ if (!vm) {
+ return 0
+ }
+ const bar = vm.$refs[LINEAR_GRADIENT_BAR] as HTMLElement | undefined
+ if (!bar) {
return 0
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getRelativePos = (points: IColorPoint) => { | |
| const bar = vm.$refs[LINEAR_GRADIENT_BAR] as HTMLElement | |
| if (!bar) { | |
| return 0 | |
| } | |
| const rect = bar.getBoundingClientRect() | |
| return Number.parseInt(((points.cursorLeft / rect.width) * 100).toFixed(0)) | |
| } | |
| const getRelativePos = (points: IColorPoint) => { | |
| if (!vm) { | |
| return 0 | |
| } | |
| const bar = vm.$refs[LINEAR_GRADIENT_BAR] as HTMLElement | undefined | |
| if (!bar) { | |
| return 0 | |
| } | |
| const rect = bar.getBoundingClientRect() | |
| return Number.parseInt(((points.cursorLeft / rect.width) * 100).toFixed(0)) | |
| } |
🤖 Prompt for AI Agents
In packages/renderless/src/color-select-panel/linear-gradient/index.ts around
lines 91 to 98, getRelativePos() accesses vm.$refs without guarding vm; add a
defensive check at the start to ensure vm and vm.$refs exist (e.g., if (!vm ||
!vm.$refs) return 0) before referencing LINEAR_GRADIENT_BAR, then proceed to get
the ref and bounding rect as before; you can also use optional chaining when
reading the ref and fall back to 0 if missing.
| const toString = () => { | ||
| const colors = context.colorPoints.value | ||
| .map((point) => { | ||
| return [point.color.value, getRelativePos(point)] as const | ||
| }) | ||
| .sort((a, b) => a[1] - b[1]) | ||
| .map(([colorValue, pos]) => { | ||
| return [colorValue, `${pos}%`].join(' ') | ||
| }) | ||
| .join(',') | ||
| return `linear-gradient(${context.deg.value}deg, ${colors})` | ||
| } | ||
| hooks.watchEffect(() => { | ||
| if (isNullOrEmpty(context.deg.value)) { | ||
| return | ||
| } | ||
| context.linearGardientValue.value = toString() | ||
| state.linearGradientBarBackground = toString().replace(`${context.deg.value}deg`, '90deg') | ||
| }) |
There was a problem hiding this comment.
Rename local toString and compute once to satisfy lint and avoid shadowing
Avoids shadowing restricted name and duplicate work inside watchEffect.
- const toString = () => {
+ const gradientToString = () => {
const colors = context.colorPoints.value
.map((point) => {
return [point.color.value, getRelativePos(point)] as const
})
.sort((a, b) => a[1] - b[1])
.map(([colorValue, pos]) => {
return [colorValue, `${pos}%`].join(' ')
})
.join(',')
return `linear-gradient(${context.deg.value}deg, ${colors})`
}
hooks.watchEffect(() => {
if (isNullOrEmpty(context.deg.value)) {
return
}
- context.linearGardientValue.value = toString()
- state.linearGradientBarBackground = toString().replace(`${context.deg.value}deg`, '90deg')
+ const s = gradientToString()
+ context.linearGardientValue.value = s
+ state.linearGradientBarBackground = s.replace(`${context.deg.value}deg`, '90deg')
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const toString = () => { | |
| const colors = context.colorPoints.value | |
| .map((point) => { | |
| return [point.color.value, getRelativePos(point)] as const | |
| }) | |
| .sort((a, b) => a[1] - b[1]) | |
| .map(([colorValue, pos]) => { | |
| return [colorValue, `${pos}%`].join(' ') | |
| }) | |
| .join(',') | |
| return `linear-gradient(${context.deg.value}deg, ${colors})` | |
| } | |
| hooks.watchEffect(() => { | |
| if (isNullOrEmpty(context.deg.value)) { | |
| return | |
| } | |
| context.linearGardientValue.value = toString() | |
| state.linearGradientBarBackground = toString().replace(`${context.deg.value}deg`, '90deg') | |
| }) | |
| const gradientToString = () => { | |
| const colors = context.colorPoints.value | |
| .map((point) => { | |
| return [point.color.value, getRelativePos(point)] as const | |
| }) | |
| .sort((a, b) => a[1] - b[1]) | |
| .map(([colorValue, pos]) => { | |
| return [colorValue, `${pos}%`].join(' ') | |
| }) | |
| .join(',') | |
| return `linear-gradient(${context.deg.value}deg, ${colors})` | |
| } | |
| hooks.watchEffect(() => { | |
| if (isNullOrEmpty(context.deg.value)) { | |
| return | |
| } | |
| const s = gradientToString() | |
| context.linearGardientValue.value = s | |
| state.linearGradientBarBackground = s.replace(`${context.deg.value}deg`, '90deg') | |
| }) |
🧰 Tools
🪛 Biome (2.1.2)
[error] 99-99: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
In packages/renderless/src/color-select-panel/linear-gradient/index.ts around
lines 99 to 117, the local function is named toString (shadowing a restricted
name) and is invoked twice inside the watchEffect causing duplicate computation;
rename the function to a non-shadowing name (e.g., buildGradientString or
gradientToString), invoke it once inside the watchEffect, store the result in a
local const, then assign context.linearGardientValue.value = result and
state.linearGradientBarBackground = result.replace(`${context.deg.value}deg`,
'90deg') so the gradient is computed only once and the restricted name is not
used.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
examples/sites/demos/apis/color-select-panel.js (2)
95-95: Prefer kebab-case for prop name in docsMulti-word props in this file use kebab-case (e.g., enable-predefine-color). Align this entry for consistency and HTML attribute usage.
- name: 'colorMode', + name: 'color-mode',
101-102: Fix English description spacing and punctuationInsert spaces around code tags and split into sentences for clarity.
- 'Determine the color mode of the color selection panel. If it is<code>linear-gradient</code>, it means it is a linear gradient If it is<code>monochrome</code>, it means monochrome selection' + 'Determine the color mode of the color selection panel. If it is <code>linear-gradient</code>, it is a linear gradient. If it is <code>monochrome</code>, it is a monochrome selection.'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/sites/demos/apis/color-select-panel.js(1 hunks)examples/sites/demos/pc/app/color-select-panel/webdoc/color-select-panel.js(1 hunks)packages/theme/src/color-select-panel/vars.less(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/theme/src/color-select-panel/vars.less
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/sites/demos/pc/app/color-select-panel/webdoc/color-select-panel.js
⏰ 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). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (2)
examples/sites/demos/apis/color-select-panel.js (2)
94-106: LGTM: New prop surface looks coherentThe addition of color mode with sensible default and demo hook is good.
104-105: Verified — demo, docs registry, Vue prop wiring, and TS typings for colorMode are present.linear-gradient.vue exists; webdoc registry lists demoId 'linear-gradient'; renderless types and chart-core expose colorMode; Vue pc component declares 'colorMode' and hue-select uses ctx.colorMode.
2 participants
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: #2515
What is the new behavior?
linear-gradient supported.
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Documentation
Style
Tests
Chores