diff --git a/src/__tests__/editor-store/ambientRule.test.ts b/src/__tests__/editor-store/ambientRule.test.ts index 09db3c050..b1611dc6d 100644 --- a/src/__tests__/editor-store/ambientRule.test.ts +++ b/src/__tests__/editor-store/ambientRule.test.ts @@ -173,6 +173,23 @@ describe('node.classIds → class attribute', () => { expect(after).toEqual(before) expect(after).not.toContain(amb.id) }) + + it('setNodeClassAssignments refuses ambient ids for a multi-selection', () => { + freshStore() + const site = useEditorStore.getState().createSite('Ambient multi-selection') + const rootId = site.pages[0].rootNodeId + const firstId = useEditorStore.getState().insertNode('base.text', {}, rootId) + const secondId = useEditorStore.getState().insertNode('base.text', {}, rootId) + const amb = useEditorStore.getState().createAmbientRule({ selector: 'h1 > span' }) + const beforeHistory = useEditorStore.getState()._historyPast.length + + useEditorStore.getState().setNodeClassAssignments([firstId, secondId], amb.id, true) + + const page = useEditorStore.getState().site!.pages[0] + expect(page.nodes[firstId].classIds ?? []).not.toContain(amb.id) + expect(page.nodes[secondId].classIds ?? []).not.toContain(amb.id) + expect(useEditorStore.getState()._historyPast.length).toBe(beforeHistory) + }) }) describe('publisher emits ambient rules', () => { diff --git a/src/__tests__/editor-store/styleRuleSlice.test.ts b/src/__tests__/editor-store/styleRuleSlice.test.ts index 3f82dedcb..fad26e549 100644 --- a/src/__tests__/editor-store/styleRuleSlice.test.ts +++ b/src/__tests__/editor-store/styleRuleSlice.test.ts @@ -4,7 +4,8 @@ * Covers: * - createClass / renameClass / deleteClass CRUD * - updateClassStyles / setClassContextStyles patch semantics - * - addNodeClass / removeNodeClass / reorderNodeClasses node assignment + * - addNodeClass / setNodeClassAssignments / removeNodeClass / + * reorderNodeClasses node assignment * - activeClassId state management * - deleteClass cascade (removes from all nodes, clears activeClassId) * - Uniqueness guards (duplicate class names throw) @@ -527,6 +528,40 @@ describe('styleRuleSlice — node class assignment', () => { expect(() => getStore().removeNodeClass(childId, cls.id)).not.toThrow() }) + it('assigns one class to multiple nodes in one undo step, without duplicates or reordering', () => { + const { rootId, childId } = setupSite() + const secondNodeId = getStore().insertNode('base.button', {}, rootId) + const existing = getStore().createClass('existing') + const target = getStore().createClass('target') + getStore().addNodeClass(childId, existing.id) + getStore().addNodeClass(secondNodeId, existing.id) + + getStore().setNodeClassAssignments([childId, secondNodeId, childId], target.id, true) + + const page = useEditorStore.getState().site!.pages[0] + expect(page.nodes[childId].classIds).toEqual([existing.id, target.id]) + expect(page.nodes[secondNodeId].classIds).toEqual([existing.id, target.id]) + + getStore().setNodeClassAssignments([childId, secondNodeId], target.id, false) + const pageAfterRemove = useEditorStore.getState().site!.pages[0] + expect(pageAfterRemove.nodes[childId].classIds).toEqual([existing.id]) + expect(pageAfterRemove.nodes[secondNodeId].classIds).toEqual([existing.id]) + + // Each batch is one undo step: removal first, then the original apply. + getStore().undo() + expect(useEditorStore.getState().site!.pages[0].nodes[childId].classIds).toEqual([ + existing.id, + target.id, + ]) + expect(useEditorStore.getState().site!.pages[0].nodes[secondNodeId].classIds).toEqual([ + existing.id, + target.id, + ]) + getStore().undo() + expect(useEditorStore.getState().site!.pages[0].nodes[childId].classIds).toEqual([existing.id]) + expect(useEditorStore.getState().site!.pages[0].nodes[secondNodeId].classIds).toEqual([existing.id]) + }) + it('reorderNodeClasses swaps positions by index', () => { const { childId } = setupSite() const cls1 = getStore().createClass('a') diff --git a/src/__tests__/panels/selectorsPanel.test.tsx b/src/__tests__/panels/selectorsPanel.test.tsx index 2c37490ec..1b325cd3e 100644 --- a/src/__tests__/panels/selectorsPanel.test.tsx +++ b/src/__tests__/panels/selectorsPanel.test.tsx @@ -736,6 +736,50 @@ describe('SelectorsPanel', () => { expect(useEditorStore.getState().site!.pages[0].nodes[textNodeId].classIds ?? []).not.toContain('cta-button') }) + it('applies and removes a selector from every selected element in one undo step', () => { + const { textNodeId, buttonNodeId } = loadSiteWithSelectors() + // Keep the anchor on the button, which already has cta-button. The other + // selected node is missing it; this is the case the old single-node gate + // incorrectly disabled/applied only to the anchor. + useEditorStore.setState({ + selectedNodeId: buttonNodeId, + selectedNodeIds: [textNodeId, buttonNodeId], + } as Parameters[0]) + render() + + const row = screen.getByRole('button', { name: /edit selector \.cta-button/i }) + fireEvent.contextMenu(row) + fireEvent.click(screen.getByRole('menuitem', { name: /apply to selected element/i })) + + const nodesAfterApply = useEditorStore.getState().site!.pages[0].nodes + expect(nodesAfterApply[textNodeId].classIds).toEqual(['hero-title', 'cta-button']) + expect(nodesAfterApply[buttonNodeId].classIds).toEqual(['hero-title', 'cta-button']) + + fireEvent.contextMenu(row) + fireEvent.click(screen.getByRole('menuitem', { name: /remove from selected element/i })) + + const nodesAfterRemove = useEditorStore.getState().site!.pages[0].nodes + expect(nodesAfterRemove[textNodeId].classIds).toEqual(['hero-title']) + expect(nodesAfterRemove[buttonNodeId].classIds).toEqual(['hero-title']) + + // The batch remove and batch apply each have their own single undo step. + useEditorStore.getState().undo() + expect(useEditorStore.getState().site!.pages[0].nodes[textNodeId].classIds).toEqual([ + 'hero-title', + 'cta-button', + ]) + expect(useEditorStore.getState().site!.pages[0].nodes[buttonNodeId].classIds).toEqual([ + 'hero-title', + 'cta-button', + ]) + useEditorStore.getState().undo() + expect(useEditorStore.getState().site!.pages[0].nodes[textNodeId].classIds).toEqual(['hero-title']) + expect(useEditorStore.getState().site!.pages[0].nodes[buttonNodeId].classIds).toEqual([ + 'hero-title', + 'cta-button', + ]) + }) + it('renames and deletes selectors with confirmation', () => { loadSiteWithSelectors() render() diff --git a/src/admin/pages/site/panels/SelectorsPanel/SelectorContextMenu.tsx b/src/admin/pages/site/panels/SelectorsPanel/SelectorContextMenu.tsx index 80f484110..6a5aefd45 100644 --- a/src/admin/pages/site/panels/SelectorsPanel/SelectorContextMenu.tsx +++ b/src/admin/pages/site/panels/SelectorsPanel/SelectorContextMenu.tsx @@ -8,8 +8,9 @@ import { PaintBucketSolidIcon } from 'pixel-art-icons/icons/paint-bucket-solid' interface SelectorContextMenuProps { x: number y: number - selectedNodeHasClass: boolean - selectedNodeId: string | null + allSelectedNodesHaveClass: boolean + anySelectedNodeHasClass: boolean + hasSelectedNodes: boolean assignable: boolean onClose: () => void onEdit: () => void @@ -25,8 +26,9 @@ interface SelectorContextMenuProps { export function SelectorContextMenu({ x, y, - selectedNodeHasClass, - selectedNodeId, + allSelectedNodesHaveClass, + anySelectedNodeHasClass, + hasSelectedNodes, assignable, onClose, onEdit, @@ -53,11 +55,11 @@ export function SelectorContextMenu({ Duplicate - + Apply to selected element - + Remove from selected element diff --git a/src/admin/pages/site/panels/SelectorsPanel/SelectorsPanel.tsx b/src/admin/pages/site/panels/SelectorsPanel/SelectorsPanel.tsx index 2da99cbd2..c3d56aa38 100644 --- a/src/admin/pages/site/panels/SelectorsPanel/SelectorsPanel.tsx +++ b/src/admin/pages/site/panels/SelectorsPanel/SelectorsPanel.tsx @@ -7,7 +7,7 @@ import { type MouseEvent, } from 'react' import { useShallow } from 'zustand/react/shallow' -import { selectSelectedNode, useEditorStore } from '@site/store/store' +import { selectActiveCanvasPage, useEditorStore } from '@site/store/store' import { classifySelectorCreateInput, styleRuleSelector } from '@core/page-tree' import { generatedClassKindLabel, isGeneratedClass, isGeneratedClassLocked } from '@core/page-tree' import type { StyleRule } from '@core/page-tree' @@ -105,8 +105,10 @@ export function SelectorsPanel({ const duplicateClass = useEditorStore((s) => s.duplicateClass) const deleteClass = useEditorStore((s) => s.deleteClass) const addNodeClass = useEditorStore((s) => s.addNodeClass) + const setNodeClassAssignments = useEditorStore((s) => s.setNodeClassAssignments) const removeNodeClass = useEditorStore((s) => s.removeNodeClass) - const selectedNode = useEditorStore(selectSelectedNode) + const activeCanvasPage = useEditorStore(selectActiveCanvasPage) + const selectedNodeIds = useEditorStore(useShallow((s) => s.selectedNodeIds)) const selectedNodeId = useEditorStore((s) => s.selectedNodeId) const activeClassId = useEditorStore((s) => s.activeClassId) @@ -164,6 +166,22 @@ export function SelectorsPanel({ const hasMore = filteredClasses.length > visibleClasses.length const selectedClass = reusableClasses.find((cls) => cls.id === selectedSelectorClassId) ?? null const contextClass = contextMenu ? site?.styleRules[contextMenu.classId] ?? null : null + // `selectedNodeIds` is the source of truth for a multi-selection. The + // selectedNodeId fallback keeps direct single-selection callers and the + // existing single-node behavior intact. + const classAssignmentNodeIds = selectedNodeIds.length > 0 + ? selectedNodeIds + : selectedNodeId + ? [selectedNodeId] + : [] + const selectedNodesHaveContextClass = contextClass + ? classAssignmentNodeIds.map((nodeId) => + Boolean(activeCanvasPage?.nodes[nodeId]?.classIds?.includes(contextClass.id)), + ) + : [] + const allSelectedNodesHaveContextClass = + selectedNodesHaveContextClass.length > 0 && selectedNodesHaveContextClass.every(Boolean) + const anySelectedNodeHasContextClass = selectedNodesHaveContextClass.some(Boolean) // The selector the Properties panel is currently editing. Mirrors that // panel's priority: an explicitly-selected selector wins, otherwise it's the @@ -280,16 +298,24 @@ export function SelectorsPanel({ } function handleApplyToSelected(cls: StyleRule) { - if (!selectedNodeId) return + if (classAssignmentNodeIds.length === 0) return if ((cls.kind ?? 'class') !== 'class') return - addNodeClass(selectedNodeId, cls.id) + if (classAssignmentNodeIds.length === 1) { + addNodeClass(classAssignmentNodeIds[0], cls.id) + } else { + setNodeClassAssignments(classAssignmentNodeIds, cls.id, true) + } setContextMenu(null) } function handleRemoveFromSelected(cls: StyleRule) { - if (!selectedNodeId) return + if (classAssignmentNodeIds.length === 0) return if ((cls.kind ?? 'class') !== 'class') return - removeNodeClass(selectedNodeId, cls.id) + if (classAssignmentNodeIds.length === 1) { + removeNodeClass(classAssignmentNodeIds[0], cls.id) + } else { + setNodeClassAssignments(classAssignmentNodeIds, cls.id, false) + } setContextMenu(null) } @@ -408,8 +434,9 @@ export function SelectorsPanel({ 0} assignable={(contextClass.kind ?? 'class') === 'class'} onClose={() => setContextMenu(null)} onEdit={() => { diff --git a/src/admin/pages/site/store/slices/styleRule/assignmentActions.ts b/src/admin/pages/site/store/slices/styleRule/assignmentActions.ts index e7a3e28cc..c1adc5a6f 100644 --- a/src/admin/pages/site/store/slices/styleRule/assignmentActions.ts +++ b/src/admin/pages/site/store/slices/styleRule/assignmentActions.ts @@ -1,11 +1,12 @@ /** * styleRule slice — node ↔ class assignment: addNodeClass, addNodeClasses, - * removeNodeClass, reorderNodeClasses, reorderNodeClass. + * setNodeClassAssignments, removeNodeClass, reorderNodeClasses, + * reorderNodeClass. * * Invariant: `node.classIds` only ever holds class-kind rule ids. Ambient * rules attach by selector matching, not by class-attribute assignment, so - * the add* actions refuse them (and log) rather than leak a never-matching - * token into the rendered `class=` attribute. + * assignment actions refuse them (and log) rather than leak a + * never-matching token into the rendered `class=` attribute. */ import type { SiteSliceHelpers } from '../site/types' @@ -16,38 +17,58 @@ type AssignmentActions = Pick< StyleRuleSlice, | 'addNodeClass' | 'addNodeClasses' + | 'setNodeClassAssignments' | 'removeNodeClass' | 'reorderNodeClasses' | 'reorderNodeClass' > export function createAssignmentActions({ get, mutateSiteState }: SiteSliceHelpers): AssignmentActions { - return { - addNodeClass(nodeId, classId) { - const { site } = get() - const node = findNodeWithClassIds(site, nodeId) - if (!node) return - // No-op if already assigned - if (node.classIds?.includes(classId)) return - // Invariant: node.classIds only holds class-kind rule ids. Ambient rules - // attach by selector matching, not by class-attribute assignment, so - // pushing one here would leak into the rendered class attribute via - // a never-matching token. Surface the misuse and bail. - const cls = site?.styleRules[classId] - if (cls && cls.kind && cls.kind !== 'class') { - console.error( - '[styleRuleSlice] addNodeClass refused: classId references an ambient rule', - { nodeId, classId, selector: cls.selector }, - ) - return - } + function updateNodeClassAssignments( + nodeIds: string[], + classId: string, + assigned: boolean, + ): void { + const { site } = get() + const uniqueNodeIds = [...new Set(nodeIds)] + if (!site || uniqueNodeIds.length === 0) return - mutateSiteState((state) => { - const mutated = mutateNodeClassIds(state, nodeId, (classIds) => { - if (!classIds.includes(classId)) classIds.push(classId) + // `kind` was added after the original class registry, so an absent kind + // remains a normal class. Ambient rules are selector-attached and must + // never enter a node's classIds array. + const cls = site.styleRules[classId] + if (cls && cls.kind && cls.kind !== 'class') { + console.error( + '[styleRuleSlice] setNodeClassAssignments refused: classId references an ambient rule', + { nodeIds: uniqueNodeIds, classId, selector: cls.selector }, + ) + return + } + + mutateSiteState((state) => { + let changed = false + for (const nodeId of uniqueNodeIds) { + mutateNodeClassIds(state, nodeId, (classIds) => { + if (assigned) { + if (classIds.includes(classId)) return + classIds.push(classId) + changed = true + return + } + + const index = classIds.indexOf(classId) + if (index === -1) return + classIds.splice(index, 1) + changed = true }) - return mutated - }) + } + return changed + }) + } + + return { + addNodeClass(nodeId, classId) { + updateNodeClassAssignments([nodeId], classId, true) }, addNodeClasses(nodeId, classIds) { @@ -81,18 +102,12 @@ export function createAssignmentActions({ get, mutateSiteState }: SiteSliceHelpe }) }, - removeNodeClass(nodeId, classId) { - const { site } = get() - const node = findNodeWithClassIds(site, nodeId) - if (!node?.classIds?.includes(classId)) return + setNodeClassAssignments(nodeIds, classId, assigned) { + updateNodeClassAssignments(nodeIds, classId, assigned) + }, - mutateSiteState((state) => { - const mutated = mutateNodeClassIds(state, nodeId, (classIds) => { - const idx = classIds.indexOf(classId) - if (idx >= 0) classIds.splice(idx, 1) - }) - return mutated - }) + removeNodeClass(nodeId, classId) { + updateNodeClassAssignments([nodeId], classId, false) }, reorderNodeClasses(nodeId, fromIndex, toIndex) { diff --git a/src/admin/pages/site/store/slices/styleRule/types.ts b/src/admin/pages/site/store/slices/styleRule/types.ts index 27535d060..980492a63 100644 --- a/src/admin/pages/site/store/slices/styleRule/types.ts +++ b/src/admin/pages/site/store/slices/styleRule/types.ts @@ -252,6 +252,14 @@ export interface StyleRuleSlice { */ addNodeClasses(nodeId: string, classIds: string[]): void + /** + * Add or remove one classId on several nodes in ONE batched mutation. This + * is the transaction boundary for applying a selector to a canvas + * multi-selection, so one action produces one undo step. Ambient rules are + * never treated as class assignments; existing class order is preserved. + */ + setNodeClassAssignments(nodeIds: string[], classId: string, assigned: boolean): void + /** Remove a classId from a node's classIds (no-op if not present). */ removeNodeClass(nodeId: string, classId: string): void