Merge pull request #347 from ArnasDon/fix/flows-add-node-crash

fix(flows): wrap add-node menu groups in DropdownMenuGroup (#336)
This commit is contained in:
Arnas Donauskas 2026-07-07 11:49:36 +03:00 committed by GitHub
commit 4e2192d84f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 95 additions and 45 deletions

View File

@ -17,7 +17,7 @@
* are list-only and have no canvas analogue.
*/
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { Fragment, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import {
CircleAlert,
Plus,
@ -39,6 +39,7 @@ import {
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuGroup,
DropdownMenuItem,
DropdownMenuLabel,
DropdownMenuSeparator,
@ -589,21 +590,27 @@ function AddNodeButton({ onAdd }: { onAdd: (type: NodeType) => void }) {
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="border-border bg-popover">
{groupNodeTypesByCategory(types).map((group, i) => (
<div key={group.id}>
// A DropdownMenuGroup (base-ui Menu.Group) is REQUIRED here:
// DropdownMenuLabel is base-ui's Menu.GroupLabel, which throws
// at render if it can't find a Menu.Group ancestor context. A
// plain <div> wrapper made opening this menu crash the page.
<Fragment key={group.id}>
{i > 0 && <DropdownMenuSeparator />}
<DropdownMenuLabel className="text-muted-foreground text-[11px] font-semibold tracking-wider uppercase">
{group.label}
</DropdownMenuLabel>
{group.types.map((t) => {
const meta = NODE_META[t];
return (
<DropdownMenuItem key={t} onClick={() => onAdd(t)}>
<meta.icon className={cn('h-3.5 w-3.5', meta.color)} />
{meta.label}
</DropdownMenuItem>
);
})}
</div>
<DropdownMenuGroup>
<DropdownMenuLabel className="text-muted-foreground text-[11px] font-semibold tracking-wider uppercase">
{group.label}
</DropdownMenuLabel>
{group.types.map((t) => {
const meta = NODE_META[t];
return (
<DropdownMenuItem key={t} onClick={() => onAdd(t)}>
<meta.icon className={cn('h-3.5 w-3.5', meta.color)} />
{meta.label}
</DropdownMenuItem>
);
})}
</DropdownMenuGroup>
</Fragment>
))}
</DropdownMenuContent>
</DropdownMenu>

View File

@ -36,7 +36,7 @@
* list view reads.
*/
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { Fragment, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import {
applyNodeChanges,
Background,
@ -87,6 +87,7 @@ import {
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuGroup,
DropdownMenuItem,
DropdownMenuLabel,
DropdownMenuSeparator,
@ -737,37 +738,43 @@ function CanvasAddNodeButton() {
className="border-border bg-popover w-[268px] p-1.5"
>
{groupNodeTypesByCategory(ADD_NODE_TYPES).map((group, i) => (
<div key={group.id}>
// DropdownMenuGroup (base-ui Menu.Group) is REQUIRED: the
// DropdownMenuLabel below is base-ui's Menu.GroupLabel, which
// throws at render without a Menu.Group ancestor. A plain <div>
// here crashed the page when this menu opened (issue #336).
<Fragment key={group.id}>
{i > 0 && <DropdownMenuSeparator />}
<DropdownMenuLabel className="text-muted-foreground px-2 py-1.5 text-[11px] font-semibold tracking-wider uppercase">
{group.label}
</DropdownMenuLabel>
{group.types.map((t) => {
const meta = NODE_META[t];
return (
<DropdownMenuItem
key={t}
onClick={() => handleAdd(t)}
className="gap-3 py-2"
>
<NodeIconChip
type={t}
size={28}
iconSize={16}
className="rounded-md"
/>
<span className="flex flex-col">
<span className="text-popover-foreground text-[13px] font-semibold">
{meta.label}
<DropdownMenuGroup>
<DropdownMenuLabel className="text-muted-foreground px-2 py-1.5 text-[11px] font-semibold tracking-wider uppercase">
{group.label}
</DropdownMenuLabel>
{group.types.map((t) => {
const meta = NODE_META[t];
return (
<DropdownMenuItem
key={t}
onClick={() => handleAdd(t)}
className="gap-3 py-2"
>
<NodeIconChip
type={t}
size={28}
iconSize={16}
className="rounded-md"
/>
<span className="flex flex-col">
<span className="text-popover-foreground text-[13px] font-semibold">
{meta.label}
</span>
<span className="text-muted-foreground text-[11.5px]">
{meta.blurb}
</span>
</span>
<span className="text-muted-foreground text-[11.5px]">
{meta.blurb}
</span>
</span>
</DropdownMenuItem>
);
})}
</div>
</DropdownMenuItem>
);
})}
</DropdownMenuGroup>
</Fragment>
))}
</DropdownMenuContent>
</DropdownMenu>

View File

@ -0,0 +1,36 @@
import { describe, it, expect } from 'vitest'
import React from 'react'
import { renderToStaticMarkup } from 'react-dom/server'
import { DropdownMenuGroup, DropdownMenuLabel } from './dropdown-menu'
/**
* Regression for issue #336 — "clicking Add node reloads the page".
*
* DropdownMenuLabel is base-ui's Menu.GroupLabel, which reads a required
* Menu.Group context and THROWS at render when it's missing. The flow
* builder's add-node menu wrapped its labels in a plain <div> instead of
* a DropdownMenuGroup, so opening the menu crashed the whole page. These
* tests pin the contract so the div-wrapper regression can't come back.
*/
describe('DropdownMenuLabel requires a DropdownMenuGroup ancestor', () => {
it('throws when rendered without a group (the #336 crash)', () => {
expect(() =>
renderToStaticMarkup(
React.createElement(DropdownMenuLabel, null, 'Messaging'),
),
).toThrow()
})
it('renders when wrapped in a DropdownMenuGroup (the fix)', () => {
expect(() =>
renderToStaticMarkup(
React.createElement(
DropdownMenuGroup,
null,
React.createElement(DropdownMenuLabel, null, 'Messaging'),
),
),
).not.toThrow()
})
})