mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-05 15:40:10 +00:00
fix: prevent duplicate context menu items by using content-based comparison (#8602)
## Summary - Switches from reference-based to content-based duplicate detection for context menu items - Fixes cases where extensions create duplicate menu items (different objects with same content) - Improves removal detection accuracy by comparing content strings instead of object references ## Details The previous implementation compared menu items by object reference, which would miss duplicates when extensions added new objects with the same content. This change: - Creates Sets of content strings from menu items for efficient duplicate detection - Filters additions by checking if their content already exists - Provides accurate count of removed items through content comparison ## Test plan - [x] Unit tests pass (`pnpm test:unit src/lib/litegraph/src/contextMenuCompat.test.ts`) - [x] TypeScript compilation succeeds (`pnpm typecheck`) - [x] Linting passes (`pnpm lint`) - [x] Pre-commit hooks pass ## Before <img width="633" height="1316" alt="Screenshot 2026-02-04 045422" src="https://github.com/user-attachments/assets/972871e2-1fd6-45a4-bb6c-9ce73ce7aed7" /> ## After <img width="531" height="854" alt="Screenshot 2026-02-04 045918" src="https://github.com/user-attachments/assets/977bed37-dfb8-41d7-b659-88c477ff8a02" /> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved context menu compatibility: more accurate detection of added/removed menu items and clearer warnings when items are removed. * **Refactor** * Updated numeric input formatting to use locale-aware formatting logic, preserving grouping and precision behavior. * **Tests** * Added a test ensuring legacy menu extraction handles items with undefined content correctly. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8602-fix-prevent-duplicate-context-menu-items-by-using-content-based-comparison-2fd6d73d36508197aa74c3409c7425fa) by [Unito](https://www.unito.io) --------- Co-authored-by: Terry Jia <terryjia88@gmail.com>
This commit is contained in:
committed by
GitHub
parent
7404756b6d
commit
3adecc4ded
@@ -195,6 +195,42 @@ describe('contextMenuCompat', () => {
|
||||
expect.any(Error)
|
||||
)
|
||||
})
|
||||
|
||||
it('should handle multiple items with undefined content correctly', () => {
|
||||
// Setup base method with items that have undefined content
|
||||
LGraphCanvas.prototype.getCanvasMenuOptions = function () {
|
||||
return [
|
||||
{ content: undefined, title: 'Separator 1' },
|
||||
{ content: undefined, title: 'Separator 2' },
|
||||
{ content: 'Item 1', callback: () => {} }
|
||||
]
|
||||
}
|
||||
|
||||
legacyMenuCompat.install(LGraphCanvas.prototype, 'getCanvasMenuOptions')
|
||||
|
||||
// Monkey-patch to add an item with undefined content
|
||||
const original = LGraphCanvas.prototype.getCanvasMenuOptions
|
||||
LGraphCanvas.prototype.getCanvasMenuOptions =
|
||||
function (): (IContextMenuValue | null)[] {
|
||||
const items = original.apply(this)
|
||||
items.push({ content: undefined, title: 'Separator 3' })
|
||||
return items
|
||||
}
|
||||
|
||||
// Extract legacy items
|
||||
const legacyItems = legacyMenuCompat.extractLegacyItems(
|
||||
'getCanvasMenuOptions',
|
||||
mockCanvas
|
||||
)
|
||||
|
||||
// Should extract only the newly added item with undefined content
|
||||
// (not collapse with existing undefined content items)
|
||||
expect(legacyItems).toHaveLength(1)
|
||||
expect(legacyItems[0]).toMatchObject({
|
||||
content: undefined,
|
||||
title: 'Separator 3'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('integration', () => {
|
||||
|
||||
@@ -152,19 +152,51 @@ class LegacyMenuCompat {
|
||||
const patchedItems = methodToCall.apply(context, args) as
|
||||
| (IContextMenuValue | null)[]
|
||||
| undefined
|
||||
if (!patchedItems) return []
|
||||
if (!patchedItems) {
|
||||
return []
|
||||
}
|
||||
// Use content-based diff to detect additions (not reference-based)
|
||||
// Create composite keys from multiple properties to handle undefined content
|
||||
const createItemKey = (item: IContextMenuValue): string => {
|
||||
const parts = [
|
||||
item.content ?? '',
|
||||
item.title ?? '',
|
||||
item.className ?? '',
|
||||
item.property ?? '',
|
||||
item.type ?? ''
|
||||
]
|
||||
return parts.join('|')
|
||||
}
|
||||
|
||||
// Use set-based diff to detect additions by reference
|
||||
const originalSet = new Set<IContextMenuValue | null>(originalItems)
|
||||
const addedItems = patchedItems.filter((item) => !originalSet.has(item))
|
||||
const originalKeys = new Set(
|
||||
originalItems
|
||||
.filter(
|
||||
(item): item is IContextMenuValue =>
|
||||
item !== null && typeof item === 'object' && 'content' in item
|
||||
)
|
||||
.map(createItemKey)
|
||||
)
|
||||
const addedItems = patchedItems.filter((item) => {
|
||||
if (item === null) return false
|
||||
if (typeof item !== 'object' || !('content' in item)) return false
|
||||
return !originalKeys.has(createItemKey(item))
|
||||
})
|
||||
|
||||
// Warn if items were removed (patched has fewer original items than expected)
|
||||
const retainedOriginalCount = patchedItems.filter((item) =>
|
||||
originalSet.has(item)
|
||||
const patchedKeys = new Set(
|
||||
patchedItems
|
||||
.filter(
|
||||
(item): item is IContextMenuValue =>
|
||||
item !== null && typeof item === 'object' && 'content' in item
|
||||
)
|
||||
.map(createItemKey)
|
||||
)
|
||||
const removedCount = [...originalKeys].filter(
|
||||
(key) => !patchedKeys.has(key)
|
||||
).length
|
||||
if (retainedOriginalCount < originalItems.length) {
|
||||
if (removedCount > 0) {
|
||||
console.warn(
|
||||
`[Context Menu Compat] Monkey patch for ${methodName} removed ${originalItems.length - retainedOriginalCount} original menu item(s). ` +
|
||||
`[Context Menu Compat] Monkey patch for ${methodName} removed ${removedCount} original menu item(s). ` +
|
||||
`This may cause unexpected behavior.`
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user