jyapayne/houserules
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(chess/ui): visual-builder drag/nesting/editing UX gaps

Browse source 
Addresses four user-reported gaps in the visual mode authoring surface:

1. × button and expand chevron triggered a drag instead of their own
   action. dnd-kit listeners were spread on the outer SortableBlockItem
   wrapper, so any pointerdown on a descendant started a sort. Fixed by
   routing only `listeners` to a new dedicated grip-handle icon on the
   card header; the rest of the card (× / expand / inspector / body)
   no longer competes with drag. `attributes` still go on the wrapper
   so keyboard drag + screen-reader announcements keep working.

2. No way to add primitives INSIDE a trigger — palette clicks always
   appended at the top level. Fixed by computing an addTargetInfo when
   the selected block is a container (has childPrimitives + a params
   .primitives array): the palette now shows an "Adding inside: {label}"
   banner with an "Add at top level instead" escape button, and
   handleAddPrimitive routes the new node into the parent params. The
   parent auto-expands and selection stays on the parent so repeated
   palette clicks stack children under it.

3. Inspector was read-only — ParamField.onChange was a documented
   no-op. Fixed by adding onParamsChange through the wire
   (VisualBuilderPane.handleParamsChange → BlockList.onParamsChange
   → SortableBlockItem.onParamsChange → BlockCard.onParamsChange →
   ParamField.onChange). Editing a number/string/enum field now
   immediately updates descriptor.primitives[index].params.

4. Nested children could not be removed — nested BlockList was given
   onRemove={() => {}}. Fixed by adding onNestedRemove to BlockListProps;
   VisualBuilderPane supplies handleNestedRemove which filters the
   matching parent params.primitives[] without mutating siblings.

Additional polish:
- Inspector now opens when a block is selected (previously needed
  selected AND expanded), so children and docs show up on first click.
- Child block list renders whenever the parent is expanded OR selected
  for the same first-click visibility.
- Expand button gains aria-label="Expand|Collapse" so accessibility
  tooling (and Playwright getByRole) can target it by name.
This commit is contained in:
Joey Yakimowich-Payne 2026-04-21 19:46:37 -06:00
commit 2d1efb1b3a
No known key found for this signature in database
3 changed files with 228 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

82
packages/chess/src/ui/visual-builder/BlockCard.tsx
View file

@ -3,6 +3,14 @@ import type { EffectPrimitiveNode, PrimitiveKind } from '../../modifiers/primiti
import { PRIMITIVE_REGISTRY } from '../../modifiers/primitives/registry.js';
import { ParamField } from '../ParamField.js';
/**
* Props that a dnd-kit `useSortable` caller hands to the drag-handle
* button. Intentionally opaque — we spread them onto the grip icon
* without caring about exact shape so dnd-kit's internal event keys
* stay encapsulated.
*/
export type DragHandleProps = Record<string, unknown>;
export interface BlockCardProps {
node: EffectPrimitiveNode;
index: number;
@ -11,8 +19,23 @@ export interface BlockCardProps {
onSelect: () => void;
onToggleExpand: () => void;
onRemove: () => void;
/**
* Called when the inspector edits the primitive's params. When
* omitted, the inspector is read-only. The caller (VisualBuilderPane)
* is responsible for producing a new descriptor with the updated
* params at this block's position in the tree.
*/
onParamsChange?: (params: unknown) => void;
depth: number; // 0..3 (visually clamped at 3)
childBlocks?: React.ReactNode;
/**
* Props from dnd-kit's `useSortable` that enable drag initiation.
* Passed through to the grip-handle button so ONLY that element
* starts a drag — clicks on the × / expand / body never do.
* Omitted when the card isn't inside a sortable context (e.g. the
* DragOverlay ghost render).
*/
dragHandleProps?: DragHandleProps;
}
const CATEGORIES: Record<string, PrimitiveKind[]> = {
@ -61,8 +84,10 @@ export default function BlockCard({
onSelect,
onToggleExpand,
onRemove,
onParamsChange,
depth,
childBlocks
childBlocks,
dragHandleProps,
}: BlockCardProps) {
const primitive = PRIMITIVE_REGISTRY.get(node.kind);
const label = primitive?.label ?? node.kind;
@ -137,6 +162,32 @@ export default function BlockCard({
{/* Header Row */}
<div className="flex items-center justify-between p-3">
<div className="flex items-center gap-3 overflow-hidden">
{dragHandleProps !== undefined && (
<button
{...dragHandleProps}
type="button"
aria-label="Drag to reorder"
title="Drag to reorder"
data-testid={`block-drag-handle-${node.kind}`}
onClick={(e) => {
// Don't select/expand when the user happens to click the
// handle. Drag initiation goes through dnd-kit's
// pointerdown listener which we spread above — a plain
// click is a no-op.
e.stopPropagation();
}}
className="cursor-grab active:cursor-grabbing touch-none select-none p-1 -ml-1 text-neutral-400 hover:text-neutral-600 focus:outline-none focus:ring-2 focus:ring-neutral-400 rounded"
>
<svg className="w-4 h-4" fill="currentColor" viewBox="0 0 20 20" aria-hidden="true">
<circle cx="7" cy="5" r="1.5" />
<circle cx="13" cy="5" r="1.5" />
<circle cx="7" cy="10" r="1.5" />
<circle cx="13" cy="10" r="1.5" />
<circle cx="7" cy="15" r="1.5" />
<circle cx="13" cy="15" r="1.5" />
</svg>
</button>
)}
<div className={`
flex items-center justify-center w-6 h-6 rounded-full text-xs font-bold shrink-0
${badgeStyles}
@ -168,7 +219,9 @@ export default function BlockCard({
: 'text-neutral-500 hover:bg-neutral-100'
}`}
title={isExpanded ? "Collapse" : "Expand"}
aria-label={isExpanded ? "Collapse" : "Expand"}
aria-expanded={isExpanded}
data-testid={`block-expand-${node.kind}`}
>
<svg
className={`w-4 h-4 transition-transform ${isExpanded ? 'rotate-180' : ''}`}
@ -196,29 +249,28 @@ export default function BlockCard({
</div>
</div>
{/* Inspector Overlay (When selected and expanded) */}
{isSelected && isExpanded && primitive && (
<div
className="px-3 pb-4 pt-1 border-t border-black/5"
onClick={e => e.stopPropagation()} // Prevent inspector clicks from triggering card select
{/* Inspector — shown whenever this block is selected so users
can always see/edit params of the primitive they picked. */}
{isSelected && primitive && (
<div
className="px-3 pb-4 pt-1 border-t border-black/5"
onClick={(e) => e.stopPropagation()}
>
<ParamField
node={node}
primitive={primitive}
allPrimitives={[node]} // We only have the single node context here for now. Good enough for standard fields.
onChange={() => {
// The visual builder itself will handle the node updates, BlockCard just renders the inspector
// Since this is read-only from the perspective of BlockCard props, we just provide a no-op
// for now until the visual builder state management (T18/T22) handles it.
// Note: For now we'll just not support direct editing via ParamField onChange within BlockCard
// to keep the API surface exactly as requested (pure presentational).
allPrimitives={[node]}
onChange={(params) => {
if (onParamsChange) onParamsChange(params);
}}
/>
</div>
)}
{/* Nested Children Area */}
{isExpanded && childBlocks && (
{/* Nested children — shown when the block is expanded OR selected,
so the user sees the inside of the trigger they just picked
from the palette without needing an extra click. */}
{(isExpanded || isSelected) && childBlocks && (
<div className="p-2 border-t border-black/5 bg-black/5 rounded-b-lg">
{childBlocks}
</div>

49
packages/chess/src/ui/visual-builder/BlockList.tsx
View file

@ -31,7 +31,16 @@ export interface BlockListProps {
onSelect: (index: number) => void;
onToggleExpand: (index: number) => void;
onRemove: (index: number) => void;
onParamsChange?: (index: number, params: unknown) => void;
onNestedReorder?: (parentIndex: number, fromChildIndex: number, toChildIndex: number) => void;
/**
* Called when the user clicks × on a child block inside a trigger.
* `parentIndex` is the child's parent in THIS list; `childIndex` is
* the child's position within `parent.params.primitives`. When
* omitted, the × button on nested blocks is a no-op (present for
* backward compat with existing callers).
*/
onNestedRemove?: (parentIndex: number, childIndex: number) => void;
depth?: number;
}
@ -44,6 +53,7 @@ interface SortableBlockItemProps {
onSelect: () => void;
onToggleExpand: () => void;
onRemove: () => void;
onParamsChange?: (params: unknown) => void;
depth: number;
childBlocks?: React.ReactNode;
}
@ -66,8 +76,16 @@ function SortableBlockItem(props: SortableBlockItemProps) {
zIndex: isDragging ? 1 : 0,
};
// Pointer-event listeners go ONLY to the grip handle rendered by
// BlockCard. `attributes` stay on the wrapper so dnd-kit's keyboard
// and screen-reader integration keeps working (role, tabIndex,
// aria-describedby, aria-pressed), but pointerdown on the card body,
// × button, expand toggle, or inspector fields no longer starts a
// drag.
const dragHandleProps = { ...listeners };
return (
<div ref={setNodeRef} style={style} {...attributes} {...listeners}>
<div ref={setNodeRef} style={style} {...attributes}>
<BlockCard
node={props.node}
index={props.index}
@ -76,8 +94,10 @@ function SortableBlockItem(props: SortableBlockItemProps) {
onSelect={props.onSelect}
onToggleExpand={props.onToggleExpand}
onRemove={props.onRemove}
{...(props.onParamsChange ? { onParamsChange: props.onParamsChange } : {})}
depth={props.depth}
childBlocks={props.childBlocks}
dragHandleProps={dragHandleProps}
/>
</div>
);
@ -91,7 +111,9 @@ export function BlockList({
onSelect,
onToggleExpand,
onRemove,
onParamsChange,
onNestedReorder,
onNestedRemove,
depth = 0,
}: BlockListProps) {
const [activeId, setActiveId] = React.useState<UniqueIdentifier | null>(null);
@ -183,25 +205,37 @@ export function BlockList({
let childBlocks: React.ReactNode = null;
if (hasChildren && isExpanded && typeof node.params === 'object' && node.params !== null && 'primitives' in node.params && Array.isArray((node.params as Record<string, unknown>).primitives)) {
// Render nested block list
const childNodes = (node.params as Record<string, unknown>).primitives as EffectPrimitiveNode[];
// Render nested block list. Selection and expansion
// are intentionally scoped to the top level for now —
// multi-level selection would need a richer path-based
// selector than the current flat number. Removal of
// individual children IS supported via onNestedRemove.
childBlocks = (
<BlockList
nodes={(node.params as Record<string, unknown>).primitives as EffectPrimitiveNode[]}
selectedIndex={null} // Nested selection not yet fully scoped, keep null for now or manage differently
expandedIndices={new Set()} // Same for nested expansion
nodes={childNodes}
selectedIndex={null}
expandedIndices={new Set()}
onReorder={(fromIdx, toIdx) => {
if (onNestedReorder) {
onNestedReorder(index, fromIdx, toIdx);
}
}}
onSelect={() => {}}
onSelect={() => {}}
onToggleExpand={() => {}}
onRemove={() => {}}
onRemove={(childIdx) => {
if (onNestedRemove) {
onNestedRemove(index, childIdx);
}
}}
depth={depth + 1}
/>
);
}
const paramsChangeProp = onParamsChange
? { onParamsChange: (params: unknown) => onParamsChange(index, params) }
: {};
return (
<SortableBlockItem
key={id}
@ -213,6 +247,7 @@ export function BlockList({
onSelect={() => onSelect(index)}
onToggleExpand={() => onToggleExpand(index)}
onRemove={() => onRemove(index)}
{...paramsChangeProp}
depth={depth}
childBlocks={childBlocks}
/>

119
packages/chess/src/ui/visual-builder/VisualBuilderPane.tsx
View file

@ -86,6 +86,35 @@ export function VisualBuilderPane({ descriptor, onChange, validationResult }: Vi
const [selectedIndex, setSelectedIndex] = useState<number | null>(null);
const [expandedIndices, setExpandedIndices] = useState<Set<number>>(new Set());
/**
* If the user has a trigger/container primitive selected (e.g. the
* user just clicked "On Turn End"), a new primitive from the palette
* lands INSIDE that trigger's `params.primitives` — not at the top
* level. Otherwise it's appended to the descriptor root.
*
* Determined by checking whether the selected primitive's registry
* entry exposes `childPrimitives` (all triggers + conditional do).
*/
const addTargetInfo = (() => {
if (selectedIndex === null) return null;
const parent = descriptor.primitives[selectedIndex];
if (!parent) return null;
const registryEntry = PRIMITIVE_REGISTRY.get(parent.kind);
if (registryEntry?.childPrimitives === undefined) return null;
if (
typeof parent.params !== 'object' ||
parent.params === null ||
!('primitives' in parent.params) ||
!Array.isArray((parent.params as Record<string, unknown>).primitives)
) {
return null;
}
return {
parentIndex: selectedIndex,
parentLabel: registryEntry.label ?? parent.kind,
};
})();
const handleAddPrimitive = (kind: PrimitiveKind) => {
const primitive = PRIMITIVE_REGISTRY.get(kind);
if (!primitive) return;
@ -95,6 +124,38 @@ export function VisualBuilderPane({ descriptor, onChange, validationResult }: Vi
params: generateDefaultParams(primitive.paramsSchema),
};
// Nested add: append to the selected container's params.primitives.
if (addTargetInfo !== null) {
const { parentIndex } = addTargetInfo;
const parent = descriptor.primitives[parentIndex];
if (!parent) return;
const parentParams = parent.params as Record<string, unknown>;
const existingChildren =
(parentParams.primitives as EffectPrimitiveNode[] | undefined) ?? [];
const newPrimitives = [...descriptor.primitives];
newPrimitives[parentIndex] = {
...parent,
params: {
...parentParams,
primitives: [...existingChildren, newNode],
},
};
onChange({ ...descriptor, primitives: newPrimitives });
// Auto-expand the parent so the new child is visible immediately.
setExpandedIndices((prev) => {
const next = new Set(prev);
next.add(parentIndex);
return next;
});
// Keep selection on the parent so successive palette clicks keep
// adding children — matches how most block editors behave.
return;
}
// Top-level add.
const newPrimitives = [...descriptor.primitives, newNode];
onChange({ ...descriptor, primitives: newPrimitives });
setSelectedIndex(newPrimitives.length - 1);
@ -170,6 +231,40 @@ export function VisualBuilderPane({ descriptor, onChange, validationResult }: Vi
onChange({ ...descriptor, primitives: newPrimitives });
};
const handleNestedRemove = (parentIdx: number, childIdx: number) => {
const parent = descriptor.primitives[parentIdx];
if (
!parent ||
typeof parent.params !== 'object' ||
parent.params === null ||
!('primitives' in parent.params)
) {
return;
}
const childPrimitives =
((parent.params as Record<string, unknown>).primitives as EffectPrimitiveNode[] | undefined) ?? [];
const filtered = childPrimitives.filter((_, i) => i !== childIdx);
const newPrimitives = [...descriptor.primitives];
newPrimitives[parentIdx] = {
...parent,
params: {
...(parent.params as Record<string, unknown>),
primitives: filtered,
},
};
onChange({ ...descriptor, primitives: newPrimitives });
};
const handleParamsChange = (index: number, params: unknown) => {
const target = descriptor.primitives[index];
if (!target) return;
const newPrimitives = [...descriptor.primitives];
newPrimitives[index] = { ...target, params };
onChange({ ...descriptor, primitives: newPrimitives });
};
const handleToggleExpand = (index: number) => {
const newExpanded = new Set(expandedIndices);
if (newExpanded.has(index)) {
@ -239,6 +334,28 @@ export function VisualBuilderPane({ descriptor, onChange, validationResult }: Vi
<div className="text-sm font-semibold text-neutral-500 uppercase tracking-wider mb-2">
Primitives
</div>
{addTargetInfo !== null && (
<div
data-testid="palette-add-target-banner"
className="mb-2 rounded-md border border-violet-300 bg-violet-50 px-3 py-2 text-xs"
>
<div className="font-semibold text-violet-900">
Adding inside:
</div>
<div className="mt-0.5 text-violet-800 truncate">
{addTargetInfo.parentLabel}
</div>
<button
type="button"
onClick={() => setSelectedIndex(null)}
className="mt-1.5 text-violet-700 underline hover:text-violet-900 focus:outline-none focus:ring-2 focus:ring-violet-400 rounded"
>
Add at top level instead
</button>
</div>
)}
{allKinds.map(renderPaletteButton)}
</div>
@ -258,7 +375,9 @@ export function VisualBuilderPane({ descriptor, onChange, validationResult }: Vi
onSelect={setSelectedIndex}
onToggleExpand={handleToggleExpand}
onRemove={handleRemove}
onParamsChange={handleParamsChange}
onNestedReorder={handleNestedReorder}
onNestedRemove={handleNestedRemove}
depth={0}
/>
)}