-
Notifications
You must be signed in to change notification settings - Fork 460
Add brush hardness commands and default keybindings for Mask Editor #7770
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: main
Are you sure you want to change the base?
Add brush hardness commands and default keybindings for Mask Editor #7770
Conversation
📝 WalkthroughWalkthroughConverted Changes
Possibly related PRs
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (7)src/**/*.{vue,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,vue,js,jsx,json,css}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (6)📚 Learning: 2025-12-17T00:40:09.635ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-12-09T03:39:54.501ZApplied to files:
📚 Learning: 2025-12-13T11:03:11.264ZApplied to files:
📚 Learning: 2025-12-30T22:22:33.836ZApplied to files:
📚 Learning: 2025-12-11T12:25:15.470ZApplied to files:
🔇 Additional comments (3)
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. Comment |
|
@Koudkunstje Have you tested the keybindings? I haven't tested your code yet, but curious if the correct syntax would be Also, we wouldn't make these changes hardcoded in coreKeybindings because of prior discussions. It should be enough to extend the settings in maskeditor.ts with the associated functions. |
Yes, I have. When you set it manually in the keybindings menu, it sets as 'shift'+'{' and if I use it in the code, it works. When I change it in the code to 'shift' + '[' , it doesn't work. Well, at least here on my mac. And about hard coding it in the keybindings. This is my first ever PR and might not have followed the desired procedures. We have to learn from it, right? But I thought I would would be handy to set by default, as these are the standard keybindings in well known photo editing software for changing the brush size and hardness. |
|
Good to hear that the 'shift'+'{' keybinding works. I think once we remove the changes made to |
Ok, I was thinking the registration of the 'shitft' + '[' and I guess similar hotkey combinations isn't correct. But that would be issue with it's own PR. |
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: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/extensions/core/maskeditor.ts
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/extensions/core/maskeditor.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/extensions/core/maskeditor.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/extensions/core/maskeditor.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/extensions/core/maskeditor.ts
🧠 Learnings (5)
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/extensions/core/maskeditor.ts
🔇 Additional comments (1)
src/extensions/core/maskeditor.ts (1)
92-106: LGTM! Command registration is correct.The brush hardness commands follow the established pattern from the existing brush size commands. The command IDs, icons, labels, and handlers are properly structured and consistent with the codebase conventions.
Minor observation: The
BrushAdjustmentSpeedsetting tooltip (line 48) mentions "brush size and hardness," but neither the existing brush size commands nor these new hardness commands apply the multiplier. This is acceptable for consistency, but consider applying the speed multiplier to both in a future enhancement.
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: 3
Fix all issues with AI Agents 🤖
In @src/extensions/core/maskeditor.ts:
- Around line 140-147: There is a duplicate declaration of changeBrushSize;
remove the second definition (the async const changeBrushSize that calls
isOpened(), useMaskEditorStore(), reads brushSettings.size and calls
store.setBrushSize) so only the original helper (the one referenced by the
commands array) remains; ensure no other code depends on the removed duplicate
and that all references point to the single changeBrushSize function.
- Around line 100-101: The decrease brush action uses _.clamp(..., 1, 100) and a
-4 step which causes asymmetric behavior vs the increase action; update the
decrease handler inside the decrease command (the function that calls
changeBrushSize) to mirror the increase logic by clamping to _.clamp(old - 2, 1,
250) so the upper limit matches 250 and the decrement step matches +2/-2
symmetry.
- Around line 109-116: The command object with id
'Comfy.MaskEditor.BrushHardness.Decrease' has a duplicate `function:` property
where the second line incorrectly calls `changeBrushSize` (a copy/paste error)
and thus overwrites the intended hardness change; remove the duplicate
`function:` line that calls `changeBrushSize`, keep a single `function: () =>
changeBrushHardness((old) => _.clamp(old - 0.1, 0, 1))` implementation for that
command, and ensure the command object ends with the proper closing brace.
♻️ Duplicate comments (1)
src/extensions/core/maskeditor.ts (1)
149-157: MovechangeBrushHardnessbefore the extension registration for consistency.While the implementation is correct, this helper should be defined alongside
changeBrushSize(after line 47) before the extension registration, not after it. This improves code organization and makes it clear these are module-level helpers used by the commands.Additionally, as noted in the previous review, both helpers should use function declarations instead of arrow functions per coding guidelines.
🔎 Proposed fix
Move this function to after line 47 and convert both helpers to function declarations:
// After line 47, before line 49: async function changeBrushSize(sizeChanger: (oldSize: number) => number) { if (!isOpened()) return const store = useMaskEditorStore() const oldBrushSize = store.brushSettings.size const newBrushSize = sizeChanger(oldBrushSize) store.setBrushSize(newBrushSize) } async function changeBrushHardness(hardnessChanger: (oldHardness: number) => number) { if (!isOpened()) return const store = useMaskEditorStore() const oldBrushHardness = store.brushSettings.hardness const newBrushHardness = hardnessChanger(oldBrushHardness) store.setBrushHardness(newBrushHardness) }Then remove the definitions at lines 40-47 and 149-157.
Based on learnings: prefer function declarations over arrow functions for pure functions.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/extensions/core/maskeditor.ts
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/extensions/core/maskeditor.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/extensions/core/maskeditor.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/extensions/core/maskeditor.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/extensions/core/maskeditor.ts
🧠 Learnings (7)
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Use function declarations instead of function expressions when possible
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Do not replace `&&=` or `||=` with `=` when there is no reason to do so. If you do find a reason to remove either `&&=` or `||=`, leave a comment explaining why the removal occurred
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/extensions/core/maskeditor.ts
🪛 Biome (2.1.2)
src/extensions/core/maskeditor.ts
[error] 114-114: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 140-140: Shouldn't redeclare 'changeBrushSize'. Consider to delete it or rename it.
'changeBrushSize' is defined here:
(lint/suspicious/noRedeclare)
benceruleanlu
left a comment
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.
there are some syntax errors with the duplicate Comfy.MaskEditor.BrushHardness.Decrease function key and a missing comma, and can you PTAL at the brush size behavior change issue that coderabbit flagged?
Matching the decrease clamp with the increase clamp for brush size
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: 1
Fix all issues with AI Agents 🤖
In @src/extensions/core/maskeditor.ts:
- Around line 102-108: The commands array references changeBrushHardness before
it's defined, causing a ReferenceError at init; move the changeBrushHardness
helper to be defined alongside changeBrushSize (i.e., place its definition
immediately after the existing changeBrushSize function and before the
app.registerExtension call so the commands array can reference it), and remove
the duplicate later definition to avoid redeclaration.
♻️ Duplicate comments (3)
src/extensions/core/maskeditor.ts (3)
109-116: Critical: Duplicatefunction:property with incorrect implementation.Lines 114 and 115 both define a
function:property. The second definition (line 115) overwrites the first and incorrectly callschangeBrushSizeinstead ofchangeBrushHardness. Remove line 115.🔎 Proposed fix
{ id: 'Comfy.MaskEditor.BrushHardness.Decrease', icon: 'pi pi-minus-circle', label: 'Decrease Brush Hardness in MaskEditor', function: () => changeBrushHardness((old) => _.clamp(old - 0.1, 0, 1)) - function: () => changeBrushSize((old) => _.clamp(old - 2, 1, 250)) },
140-147: Critical: Remove duplicatechangeBrushSizedeclaration.This is an exact duplicate of the
changeBrushSizehelper already defined at lines 40-47. The commands array (lines 77-128) references the first definition, so this redeclaration serves no purpose and violates the no-redeclare lint rule.🔎 Proposed fix
- -const changeBrushSize = async (sizeChanger: (oldSize: number) => number) => { - if (!isOpened()) return - - const store = useMaskEditorStore() - const oldBrushSize = store.brushSettings.size - const newBrushSize = sizeChanger(oldBrushSize) - store.setBrushSize(newBrushSize) -} -
149-157: Refactor to function declaration and move before usage.After moving this helper to before line 49 (as noted in the lines 102-108 comment), refactor both
changeBrushSizeandchangeBrushHardnessto use function declarations instead of arrow functions for consistency with the project's coding guidelines.🔎 Proposed refactor
After moving to the correct location (lines 40-57), apply this refactor:
-const changeBrushSize = async (sizeChanger: (oldSize: number) => number) => { +async function changeBrushSize(sizeChanger: (oldSize: number) => number) { if (!isOpened()) return const store = useMaskEditorStore() const oldBrushSize = store.brushSettings.size const newBrushSize = sizeChanger(oldBrushSize) store.setBrushSize(newBrushSize) -} +} -const changeBrushHardness = async (hardnessChanger: (oldHardness: number) => number) => { +async function changeBrushHardness(hardnessChanger: (oldHardness: number) => number) { if (!isOpened()) return const store = useMaskEditorStore() const oldBrushHardness = store.brushSettings.hardness const newBrushHardness = hardnessChanger(oldBrushHardness) store.setBrushHardness(newBrushHardness) -} +}Based on learnings, function declarations are preferred over function expressions for pure functions in this repository.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/extensions/core/maskeditor.ts
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/extensions/core/maskeditor.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/extensions/core/maskeditor.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/extensions/core/maskeditor.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/extensions/core/maskeditor.ts
🧠 Learnings (7)
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Use function declarations instead of function expressions when possible
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Do not replace `&&=` or `||=` with `=` when there is no reason to do so. If you do find a reason to remove either `&&=` or `||=`, leave a comment explaining why the removal occurred
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/extensions/core/maskeditor.ts
🪛 Biome (2.1.2)
src/extensions/core/maskeditor.ts
[error] 115-115: expected , but instead found function
Remove function
(parse)
[error] 114-114: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 140-140: Shouldn't redeclare 'changeBrushSize'. Consider to delete it or rename it.
'changeBrushSize' is defined here:
(lint/suspicious/noRedeclare)
that brush size function shouldn't be in the brush hardness command.
Can someone help me, as this is my first ever PR and new to me? This is kinda overwhelming to me. Maybe I bit off more than I can chew. With the code changes from merging with the main, all kinds of new issues arise. Can someone help me one of these days on discord and help me solve these issues one by one? So, I can learn from all of this? |
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
I've restored the coreKeybindings to it's original state. |
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: 1
🤖 Fix all issues with AI Agents
In @package.json:
- Around line 71-72: The PR claims "no new dependencies" but devDependencies now
include "@swc-node/register", "@swc/core", and "ts-node"; either update the PR
description to state that these devDependencies were added (and note they are
peer deps required by existing Nx tooling), or remove them if they were added
unintentionally; locate the devDependencies block and the PR text referencing
"no new dependencies" and ensure the description accurately reflects the
inclusion of "@swc-node/register", "@swc/core", and "ts-node" (or revert those
entries) and add a short rationale that they are peer dependencies of existing
tools if keeping them.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
package.json
🧠 Learnings (10)
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Use sorted and grouped imports organized by plugin/source
Applied to files:
package.json
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Use es-toolkit for utility functions
Applied to files:
package.json
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Applied to files:
package.json
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Run ESLint instead of manually figuring out whitespace fixes or other trivial style concerns using the `pnpm lint:fix` command
Applied to files:
package.json
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.ts : Use es-toolkit for utility functions
Applied to files:
package.json
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Use es-toolkit for utility functions instead of other utility libraries
Applied to files:
package.json
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue,js,jsx,json,css} : Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Applied to files:
package.json
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Use TypeScript exclusively; do not write new JavaScript code
Applied to files:
package.json
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{vue,ts,tsx} : Follow Vue 3 composition API style guide
Applied to files:
package.json
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx} : Minimize the surface area (exported values) of each module and composable
Applied to files:
package.json
2ddec88 to
b59b4fe
Compare
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: 1
🤖 Fix all issues with AI agents
In @src/extensions/core/maskeditor.ts:
- Around line 113-126: The brush hardness commands are correct; remove the
redundant inline comments before the two command objects for Increase and
Decrease (the objects with id 'Comfy.MaskEditor.BrushHardness.Increase' and
'Comfy.MaskEditor.BrushHardness.Decrease') so the file follows the same
self-documenting style as the brush size commands, leaving the id, label, icon
and the function that calls changeBrushHardness((old) => _.clamp(...))
unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/extensions/core/maskeditor.ts
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/extensions/core/maskeditor.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/extensions/core/maskeditor.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/extensions/core/maskeditor.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
src/extensions/core/maskeditor.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/extensions/core/maskeditor.ts
🧠 Learnings (6)
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Do not replace `&&=` or `||=` with `=` when there is no reason to do so. If you do find a reason to remove either `&&=` or `||=`, leave a comment explaining why the removal occurred
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/extensions/core/maskeditor.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/extensions/core/maskeditor.ts
🧬 Code graph analysis (1)
src/extensions/core/maskeditor.ts (1)
src/stores/maskEditorStore.ts (1)
useMaskEditorStore(19-278)
🔇 Additional comments (2)
src/extensions/core/maskeditor.ts (2)
40-47: LGTM! Function declaration refactoring completed correctly.The refactoring from arrow function to function declaration follows the project's coding guidelines and maintains all the original logic correctly.
Based on learnings, function declarations are preferred over function expressions for pure functions in this repository.
49-58: LGTM! New brush hardness helper correctly mirrors brush size pattern.The
changeBrushHardnessfunction follows the same defensive pattern aschangeBrushSizewith theisOpened()guard and correctly integrates with the store'ssetBrushHardnessmethod (which clamps values to [0, 1]).
…nstje/ComfyUI_frontend into brush-hardness-keybinding
Removed comments for brush size and hardness commands.

Summary
Added brush hardness commands and default keybindings for the Mask Editor to match Photoshop behavior.
Changes
What:
Implemented Increase and Decrease brush hardness commands in maskeditor.ts
Added default keybindings for brush size ([ / ]) and brush hardness (Shift + { / Shift + }) in coreKeybindings.ts
Exposed brush hardness entries in the keybinding menu
Breaking: None
Dependencies: None
Review Focus
Screenshots (if applicable)
┆Issue is synchronized with this Notion page by Unito