mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-11 16:30:57 +00:00
feat: sort right-click context menu categories alphabetically (#12039)
*PR Created by the Glary-Bot Agent*
---
## Summary
Sort the canvas right-click "Add Node" context menu by display name
(case-insensitive, natural numeric). Previously, both category submenus
and leaf nodes appeared in node-registration order, making the menu hard
to scan for users browsing for nodes.
This change is scoped specifically to the **smaller right-click
contextual menu**. It does NOT affect the double-click search menu or
the left-side Nodes panel.
## Changes
- `src/lib/litegraph/src/LGraphCanvas.ts` — In `onMenuAdd` →
`inner_onMenuAdded`, sort the deduplicated category submenu entries and
the leaf-node entries by `content` using `localeCompare` with `{
numeric: true, sensitivity: 'base' }`. Categories still appear before
leaf nodes within a level (preserves existing UX).
- `src/lib/litegraph/src/LGraphCanvas.onMenuAdd.test.ts` — New unit
tests that mock `LiteGraph.ContextMenu` and assert: case-insensitive
sort, natural numeric ordering (`Cat1`, `Cat2`, `Cat10`), leaf-node
sorting inside a category, and category-before-leaf placement.
## Verification
- `pnpm vitest run src/lib/litegraph/src/LGraphCanvas.onMenuAdd.test.ts`
— 4/4 pass
- `pnpm typecheck` — clean (ran via pre-commit hook on initial commit)
- `oxfmt` / `oxlint` / `eslint` — clean
- Oracle review against `main` returned 0 critical / 1 warning (test
coverage) / 1 suggestion (numeric sort) — both addressed in this PR.
## Notes
- The sort is applied at the menu-build site rather than inside
`LiteGraphGlobal.getNodeTypesCategories`/`getNodeTypesInCategory` to
keep the change scoped to the menu UX and avoid changing the iteration
order seen by extensions that consume those public methods.
- Per user request, this is opening as a draft PR for self-review +
CodeRabbit feedback in a single follow-up pass; manual browser
verification (right-click screenshots) was deferred to that pass.
- Slack thread context: user reported the contextual menu is "a mess"
for discovering native nodes; alphabetical sorting addresses the
discoverability problem without touching the search-oriented menus.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12039-feat-sort-right-click-context-menu-categories-alphabetically-3596d73d36508107a87ffec1c353994e)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: Alexis Rolland <alexis@comfy.org>
This commit is contained in:
173
src/lib/litegraph/src/LGraphCanvas.onMenuAdd.test.ts
Normal file
173
src/lib/litegraph/src/LGraphCanvas.onMenuAdd.test.ts
Normal file
@@ -0,0 +1,173 @@
|
||||
import { fromPartial } from '@total-typescript/shoehorn'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { IContextMenuValue } from '@/lib/litegraph/src/litegraph'
|
||||
import {
|
||||
LGraph,
|
||||
LGraphCanvas,
|
||||
LGraphNode,
|
||||
LiteGraph
|
||||
} from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
class TestNode extends LGraphNode {
|
||||
static override type = 'TestNode'
|
||||
|
||||
constructor(title?: string) {
|
||||
super(title ?? 'TestNode')
|
||||
}
|
||||
}
|
||||
|
||||
function makeNodeClass(title: string) {
|
||||
class N extends TestNode {
|
||||
static override title = title
|
||||
|
||||
constructor() {
|
||||
super(title)
|
||||
}
|
||||
}
|
||||
return N
|
||||
}
|
||||
|
||||
function createCanvas(graph: LGraph): LGraphCanvas {
|
||||
const el = document.createElement('canvas')
|
||||
el.width = 800
|
||||
el.height = 600
|
||||
const ctx = fromPartial<CanvasRenderingContext2D>({
|
||||
measureText: vi.fn().mockReturnValue({ width: 50 }),
|
||||
getTransform: vi
|
||||
.fn()
|
||||
.mockReturnValue({ a: 1, b: 0, c: 0, d: 1, e: 0, f: 0 })
|
||||
})
|
||||
|
||||
el.getContext = vi.fn().mockReturnValue(ctx)
|
||||
el.getBoundingClientRect = vi.fn().mockReturnValue({
|
||||
left: 0,
|
||||
top: 0,
|
||||
width: 800,
|
||||
height: 600
|
||||
})
|
||||
|
||||
return new LGraphCanvas(el, graph, { skip_render: true })
|
||||
}
|
||||
|
||||
type MenuEntry = IContextMenuValue<string>
|
||||
|
||||
describe('LGraphCanvas.onMenuAdd category sorting', () => {
|
||||
let graph: LGraph
|
||||
let canvas: LGraphCanvas
|
||||
const registeredTypes: string[] = []
|
||||
let originalContextMenu: typeof LiteGraph.ContextMenu
|
||||
const capturedEntries: MenuEntry[][] = []
|
||||
|
||||
beforeEach(() => {
|
||||
graph = new LGraph()
|
||||
canvas = createCanvas(graph)
|
||||
LGraphCanvas.active_canvas = canvas
|
||||
|
||||
capturedEntries.length = 0
|
||||
originalContextMenu = LiteGraph.ContextMenu
|
||||
const MockContextMenu = vi.fn(function (
|
||||
this: unknown,
|
||||
values: MenuEntry[]
|
||||
) {
|
||||
capturedEntries.push(values)
|
||||
}) as unknown as typeof LiteGraph.ContextMenu
|
||||
LiteGraph.ContextMenu = MockContextMenu
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
LiteGraph.ContextMenu = originalContextMenu
|
||||
for (const type of registeredTypes) {
|
||||
delete LiteGraph.registered_node_types[type]
|
||||
}
|
||||
registeredTypes.length = 0
|
||||
})
|
||||
|
||||
function register(type: string, title: string) {
|
||||
LiteGraph.registerNodeType(type, makeNodeClass(title))
|
||||
registeredTypes.push(type)
|
||||
}
|
||||
|
||||
function openTopLevelMenu() {
|
||||
const event = new MouseEvent('contextmenu', { clientX: 10, clientY: 10 })
|
||||
LGraphCanvas.onMenuAdd(undefined, undefined, event)
|
||||
return event
|
||||
}
|
||||
|
||||
function drillInto(label: string, sourceEvent: MouseEvent) {
|
||||
const top = capturedEntries[capturedEntries.length - 1]
|
||||
const entry = top.find((e) => e.content === label)
|
||||
expect(entry, `submenu entry "${label}" should exist`).toBeDefined()
|
||||
expect(entry!.callback).toBeDefined()
|
||||
expect(typeof entry!.value).toBe('string')
|
||||
const callback = entry!.callback!
|
||||
const menuThis = document.createElement('div') as ThisParameterType<
|
||||
typeof callback
|
||||
>
|
||||
void callback.call(menuThis, entry, undefined, sourceEvent, undefined)
|
||||
}
|
||||
|
||||
it('sorts top-level category submenus alphabetically (case-insensitive)', () => {
|
||||
register('zebra/zNode', 'Zebra Node')
|
||||
register('Apple/aNode', 'Apple Node')
|
||||
register('middle/mNode', 'Middle Node')
|
||||
|
||||
openTopLevelMenu()
|
||||
|
||||
const submenuLabels = capturedEntries[0]
|
||||
.filter((e) => e.has_submenu)
|
||||
.map((e) => e.content)
|
||||
const ours = submenuLabels.filter((label) =>
|
||||
['Apple', 'middle', 'zebra'].includes(label ?? '')
|
||||
)
|
||||
expect(ours).toEqual(['Apple', 'middle', 'zebra'])
|
||||
})
|
||||
|
||||
it('uses natural numeric ordering for numbered category names', () => {
|
||||
register('Cat10/n10', 'Item10')
|
||||
register('Cat2/n2', 'Item2')
|
||||
register('Cat1/n1', 'Item1')
|
||||
|
||||
openTopLevelMenu()
|
||||
|
||||
const ours = capturedEntries[0]
|
||||
.filter(
|
||||
(e) =>
|
||||
e.has_submenu && ['Cat1', 'Cat2', 'Cat10'].includes(e.content ?? '')
|
||||
)
|
||||
.map((e) => e.content)
|
||||
expect(ours).toEqual(['Cat1', 'Cat2', 'Cat10'])
|
||||
})
|
||||
|
||||
it('sorts leaf nodes inside a category alphabetically', () => {
|
||||
register('leafsort/Zeta', 'Zeta')
|
||||
register('leafsort/Alpha', 'Alpha')
|
||||
register('leafsort/Mike', 'Mike')
|
||||
|
||||
const event = openTopLevelMenu()
|
||||
drillInto('leafsort', event)
|
||||
|
||||
const leafLabels = capturedEntries[1]
|
||||
.filter((e) => !e.has_submenu)
|
||||
.map((e) => e.content)
|
||||
expect(leafLabels).toEqual(['Alpha', 'Mike', 'Zeta'])
|
||||
})
|
||||
|
||||
it('places category submenus before leaf entries within a category level', () => {
|
||||
register('mixed/leafA', 'A Leaf')
|
||||
register('mixed/leafZ', 'Z Leaf')
|
||||
register('mixed/inner/deep', 'Deep')
|
||||
|
||||
const event = openTopLevelMenu()
|
||||
drillInto('mixed', event)
|
||||
|
||||
const inside = capturedEntries[1]
|
||||
const ours = inside.filter((e) =>
|
||||
['inner', 'A Leaf', 'Z Leaf'].includes(e.content ?? '')
|
||||
)
|
||||
expect(ours[0].content).toBe('inner')
|
||||
expect(ours[0].has_submenu).toBe(true)
|
||||
expect(ours[1].content).toBe('A Leaf')
|
||||
expect(ours[2].content).toBe('Z Leaf')
|
||||
})
|
||||
})
|
||||
@@ -1179,7 +1179,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
const categories = LiteGraph.getNodeTypesCategories(
|
||||
canvas.filter || graph.filter
|
||||
).filter((category) => category.startsWith(base_category))
|
||||
const entries: AddNodeMenu[] = []
|
||||
const categoryEntries: AddNodeMenu[] = []
|
||||
|
||||
for (const category of categories) {
|
||||
if (!category) continue
|
||||
@@ -1197,11 +1197,11 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
// in case it has a namespace like "shader::math/rand" it hides the namespace
|
||||
if (name.includes('::')) name = name.split('::', 2)[1]
|
||||
|
||||
const index = entries.findIndex(
|
||||
const index = categoryEntries.findIndex(
|
||||
(entry) => entry.value === category_path
|
||||
)
|
||||
if (index === -1) {
|
||||
entries.push({
|
||||
categoryEntries.push({
|
||||
value: category_path,
|
||||
content: name,
|
||||
has_submenu: true,
|
||||
@@ -1212,11 +1212,19 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
}
|
||||
}
|
||||
|
||||
const compareByContent = (a: AddNodeMenu, b: AddNodeMenu) =>
|
||||
(a.content ?? '').localeCompare(b.content ?? '', undefined, {
|
||||
numeric: true,
|
||||
sensitivity: 'base'
|
||||
})
|
||||
categoryEntries.sort(compareByContent)
|
||||
|
||||
const nodes = LiteGraph.getNodeTypesInCategory(
|
||||
base_category.slice(0, -1),
|
||||
canvas.filter || graph.filter
|
||||
)
|
||||
|
||||
const nodeEntries: AddNodeMenu[] = []
|
||||
for (const node of nodes) {
|
||||
if (node.skip_list) continue
|
||||
|
||||
@@ -1246,9 +1254,13 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
}
|
||||
}
|
||||
|
||||
entries.push(entry)
|
||||
nodeEntries.push(entry)
|
||||
}
|
||||
|
||||
nodeEntries.sort(compareByContent)
|
||||
|
||||
const entries: AddNodeMenu[] = [...categoryEntries, ...nodeEntries]
|
||||
|
||||
new LiteGraph.ContextMenu(entries, { event: e, parentMenu: prev_menu })
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user