Comfy-Org / Comfy-Org/ComfyUI_frontend
Refactor: Consolidate node color picker implementations
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 697
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 495
Description
## Overview
Explore consolidating the node color picker functionality across multiple implementations to reduce code duplication and improve maintainability.
## Context
This issue was created during review of PR #7812, which introduces a new `SetNodeColor.vue` component for the right-side panel settings.
**Related PR:** #7812
**Comment:** https://github.com/Comfy-Org/ComfyUI_frontend/pull/7812#discussion_r2685105004
**Requested by:** @christian-byrne
## Current State
Multiple color picker implementations exist across the codebase with significant code duplication:
1. **src/components/rightSidePanel/settings/SetNodeColor.vue** (new in PR #7812)
- Lines 39-62: Color options generation from `LGraphCanvas.node_colors`
- Lines 68-103: Color option getter/setter logic
- Lines 41-47: `getColorValue` function for color adjustments
2. **src/components/graph/selectionToolbox/ColorPickerButton.vue**
- Lines 95-100: Nearly identical color options generation
- Lines 103-120: Similar color option getter/setter logic
- Uses same `NO_COLOR_OPTION` pattern
3. **src/composables/graph/useNodeCustomization.ts**
- Lines 57-73: Color options generation
- Lines 85-100: Color customization logic
- Lines 125-135: Current color option retrieval
4. **src/components/common/FormColorPicker.vue**
5. **src/components/common/ColorCustomizationSelector.vue**
6. **src/renderer/extensions/vueNodes/widgets/components/WidgetColorPicker.vue**
## Duplicated Logic
### Color Options Generation
All implementations use similar logic to generate color options from `LGraphCanvas.node_colors`:
```typescript
const colorOptions = [
NO_COLOR_OPTION,
...Object.entries(LGraphCanvas.node_colors).map(([name, color]) => ({
name,
localizedName: t(\`color.\${name}\`),
value: getColorValue(color.bgcolor)
}))
]
```
### Color Application
All use the same `IColorable` interface methods:
- `item.getColorOption()`
- `item.setColorOption(colorOption)`
### Color Value Processing
Multiple implementations compute color variants (dark/light theme adjustments) using `adjustColor`.
## Proposed Solutions
### Option 1: Extract to Composable
Create a shared composable (e.g., `useNodeColorPicker`) that provides:
- `colorOptions` (computed from `LGraphCanvas.node_colors`)
- `getColorValue` utility
- Helper functions for getting/setting colors on `IColorable` items
### Option 2: Shared Utility Module
Create `src/utils/nodeColorUtil.ts` with:
- `generateColorOptions()`
- `getColorValue(color: string)`
- `applyColorToItems(items: IColorable[], colorName: string)`
- `getCurrentColor(items: IColorable[])`
### Option 3: Base Color Picker Component
Create a reusable `BaseNodeColorPicker.vue` component that can be configured for different contexts (selection toolbox, right panel, widgets, etc.)
## Benefits
- Reduce code duplication (~100+ lines of duplicated logic)
- Single source of truth for color option generation
- Easier to maintain and test
- Consistent color picker behavior across the UI
- Simpler to add new color-related features
## Files to Refactor
- `src/components/rightSidePanel/settings/SetNodeColor.vue`
- `src/components/graph/selectionToolbox/ColorPickerButton.vue`
- `src/composables/graph/useNodeCustomization.ts`
- `src/components/common/FormColorPicker.vue`
- `src/components/common/ColorCustomizationSelector.vue`
- `src/renderer/extensions/vueNodes/widgets/components/WidgetColorPicker.vue`
## Additional Considerations
- Review if `src/utils/litegraphUtil.ts` (which already has some color-related utilities) should be extended
- Consider theme-aware color value computation
- Ensure backward compatibility with existing implementations
- Update related tests
## Acceptance Criteria
- [ ] Identify all color picker implementations
- [ ] Design shared API/interface
- [ ] Implement consolidated solution
- [ ] Refactor existing implementations to use shared code
- [ ] Add/update tests
- [ ] Verify no regressions in color picker functionality
- [ ] Update documentation if needed
┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-8024-Refactor-Consolidate-node-color-picker-implementations-2e76d73d365081fda6a4eb7f269a5cf9) by [Unito](https://www.unito.io)
Contributor guide
Assessment
This issue has not been assessed yet.