From 0300cb521be1d62eceabc1baed3f00d53a6aa8cd Mon Sep 17 00:00:00 2001 From: sadkodev Date: Fri, 17 Jul 2026 14:09:40 -0400 Subject: [PATCH] refactor(keyboard): address review feedback on opacity shortcuts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move setOpacity to core editor action (packages/core/src/editor/nodes.ts) so it is testable directly without duplicating logic in tests - Replace external window listener with tinykeys bindings in registerKeyboardShortcuts, reusing shared input/focus guards - Add 20 bindings for Digit0-9 and Numpad0-9 with modifier guards - Implement Figma-style multi-digit buffer: 2→20%, 28→28%, 00→0%, 100→100% with 400ms timeout and auto-apply at 3 digits - Simplify selection.setOpacity command to delegate to editor.setOpacity - Rewrite unit tests to call editor.setOpacity directly - Add ofetch to dependencies (was imported but missing from package.json) - Update CHANGELOG --- CHANGELOG.md | 2 +- bun.lock | 1 + package.json | 1 + packages/core/src/editor/nodes.ts | 15 ++++++ packages/vue/src/editor/commands/selection.ts | 12 +---- src/app/shell/keyboard/actions.ts | 32 +++++++++++- src/app/shell/keyboard/opacity.ts | 52 ------------------- src/app/shell/keyboard/registry.ts | 26 +++++++++- src/app/shell/keyboard/types.ts | 1 + src/app/shell/keyboard/use.ts | 10 ++-- tests/engine/editor/opacity.test.ts | 49 ++++++++++------- 11 files changed, 112 insertions(+), 89 deletions(-) delete mode 100644 src/app/shell/keyboard/opacity.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d36487dd6..aae39da6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Changed -- Add Figma-style opacity keyboard shortcuts (`1`-`9` for 10%-90%, `0` for 100%) on the selected layers, with undo-batched multi-selection support. +- Add Figma-style opacity keyboard shortcuts (`1`-`9` for 10%-90%, `0` for 100%) with multi-digit buffer support (`28` → 28%, `00` → 0%, `100` → 100%), undo-batched multi-selection, and both top-row and numpad digit support. - Add Figma-style page management in the Pages panel, including rename/delete actions and drag-and-drop page reordering. - Add DOM/CSS import and authoring support so HTML, CSS, Tailwind, and JSX can be converted into editable OpenPencil documents from the app, CLI, and SDK. - Add Tailwind class serialization for DOM/CSS HTML export in the SDK and CLI. diff --git a/bun.lock b/bun.lock index be1e55a26..e84aa2a06 100644 --- a/bun.lock +++ b/bun.lock @@ -44,6 +44,7 @@ "jspdf": "^4.2.1", "lib0": "^0.2.117", "motion-v": "^2.0.0", + "ofetch": "^1.5.1", "opentype.js": "^2.0.0", "prismjs": "^1.30.0", "reka-ui": "^2.9.0", diff --git a/package.json b/package.json index 38abddb32..51a0a7e75 100644 --- a/package.json +++ b/package.json @@ -97,6 +97,7 @@ "jspdf": "^4.2.1", "lib0": "^0.2.117", "motion-v": "^2.0.0", + "ofetch": "^1.5.1", "opentype.js": "^2.0.0", "prismjs": "^1.30.0", "reka-ui": "^2.9.0", diff --git a/packages/core/src/editor/nodes.ts b/packages/core/src/editor/nodes.ts index f00fa443c..b4042272b 100644 --- a/packages/core/src/editor/nodes.ts +++ b/packages/core/src/editor/nodes.ts @@ -51,9 +51,24 @@ export function createNodeActions(ctx: EditorContext) { ctx.requestRender() } + function setOpacity(opacity: number) { + const clamped = Math.max(0, Math.min(1, opacity)) + const ids = [...ctx.state.selectedIds] + if (ids.length === 0) return + const targets = ids.map((id) => ctx.graph.getNode(id)).filter((n): n is SceneNode => n != null) + const changed = targets.filter((t) => t.opacity !== clamped) + if (changed.length === 0) return + ctx.undo.runBatch('Set opacity', () => { + for (const target of changed) { + updateNodeWithUndo(target.id, { opacity: clamped }, 'Set opacity') + } + }) + } + return { updateNode, updateNodeWithUndo, + setOpacity, ...layoutModeActions, ...variableBindingActions, ...nudgeActions diff --git a/packages/vue/src/editor/commands/selection.ts b/packages/vue/src/editor/commands/selection.ts index 8bcf70a32..6753b53ef 100644 --- a/packages/vue/src/editor/commands/selection.ts +++ b/packages/vue/src/editor/commands/selection.ts @@ -269,17 +269,7 @@ export function createSelectionCommands({ return t.value.setOpacity }, enabled: capabilities.canSetOpacity, - run: () => { - const opacity = getOpacityTarget() - const targets = editor.getSelectedNodes() - if (targets.length === 0) return - editor.undo.runBatch('Set opacity', () => { - for (const target of targets) { - if (target.opacity === opacity) continue - editor.updateNodeWithUndo(target.id, { opacity }, 'Set opacity') - } - }) - } + run: () => editor.setOpacity(getOpacityTarget()) } } } diff --git a/src/app/shell/keyboard/actions.ts b/src/app/shell/keyboard/actions.ts index 5d7f7c4e9..95143200a 100644 --- a/src/app/shell/keyboard/actions.ts +++ b/src/app/shell/keyboard/actions.ts @@ -9,13 +9,15 @@ type KeyboardActionsOptions = { activeTab: Ref<'design' | 'code' | 'ai'> isMobile: ReturnType['isMobile'] runCommand: ReturnType['runCommand'] + setOpacityTarget: ReturnType['setOpacityTarget'] } export function createKeyboardActions({ store, activeTab, isMobile, - runCommand + runCommand, + setOpacityTarget }: KeyboardActionsOptions) { function hasNodeEditSelection() { return ( @@ -98,6 +100,31 @@ export function createKeyboardActions({ if (store.state.selectedIds.size > 0) void store.exportSelection(1, 'png') } + let opacityBuffer = '' + let opacityTimer: ReturnType | undefined + + function applyOpacityBuffer() { + clearTimeout(opacityTimer) + if (!opacityBuffer) return + const n = Number.parseInt(opacityBuffer, 10) + const percent = opacityBuffer.length === 1 ? n * 10 : n + const clamped = Math.min(100, Math.max(0, percent)) + opacityBuffer = '' + setOpacityTarget(clamped / 100) + runCommand('selection.setOpacity') + } + + function opacityDigit(digit: string) { + if (store.state.selectedIds.size === 0) return + opacityBuffer += digit + clearTimeout(opacityTimer) + if (opacityBuffer.length >= 3) { + applyOpacityBuffer() + } else { + opacityTimer = setTimeout(applyOpacityBuffer, 400) + } + } + return { smartDelete, confirmOrEnterText, @@ -105,6 +132,7 @@ export function createKeyboardActions({ toggleAutoLayout, toggleUI, toggleAI, - exportSelectionPng + exportSelectionPng, + opacityDigit } } diff --git a/src/app/shell/keyboard/opacity.ts b/src/app/shell/keyboard/opacity.ts deleted file mode 100644 index 13c846108..000000000 --- a/src/app/shell/keyboard/opacity.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { useEventListener } from '@vueuse/core' - -import type { useEditorCommands } from '@open-pencil/vue' - -import type { EditorStore } from '@/app/editor/active-store' -import { isEditing } from '@/app/shell/keyboard/focus' - -type OpacityShortcutOptions = { - store: EditorStore - setOpacityTarget: ReturnType['setOpacityTarget'] - runCommand: ReturnType['runCommand'] -} - -const DIGIT_OPACITY = { - Digit0: 1, - Numpad0: 1, - Digit1: 0.1, - Numpad1: 0.1, - Digit2: 0.2, - Numpad2: 0.2, - Digit3: 0.3, - Numpad3: 0.3, - Digit4: 0.4, - Numpad4: 0.4, - Digit5: 0.5, - Numpad5: 0.5, - Digit6: 0.6, - Numpad6: 0.6, - Digit7: 0.7, - Numpad7: 0.7, - Digit8: 0.8, - Numpad8: 0.8, - Digit9: 0.9, - Numpad9: 0.9 -} as const - -export function bindOpacityKeys({ store, setOpacityTarget, runCommand }: OpacityShortcutOptions) { - useEventListener(window, 'keydown', (e: KeyboardEvent) => { - if (isEditing(e)) return - if (store.state.editingTextId) return - if (store.state.numberFieldFocused) return - if (e.metaKey || e.ctrlKey || e.altKey || e.shiftKey) return - if (store.state.selectedIds.size === 0) return - - if (!(e.code in DIGIT_OPACITY)) return - const opacity: number = DIGIT_OPACITY[e.code as keyof typeof DIGIT_OPACITY] - - e.preventDefault() - setOpacityTarget(opacity) - runCommand('selection.setOpacity') - }) -} diff --git a/src/app/shell/keyboard/registry.ts b/src/app/shell/keyboard/registry.ts index c6ba53d29..69f907f46 100644 --- a/src/app/shell/keyboard/registry.ts +++ b/src/app/shell/keyboard/registry.ts @@ -36,6 +36,29 @@ function commandShortcuts(...commands: EditorCommandId[]): ShortcutDefinition[] }) } +const OPACITY_DIGITS = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'] + +function opacityBindings(): ShortcutDefinition[] { + return OPACITY_DIGITS.flatMap((digit) => [ + { + id: `opacity-Digit${digit}`, + keys: `Digit${digit}`, + run: ({ keyEvent, actions }: KeyboardShortcutRunOptions) => { + if (keyEvent.metaKey || keyEvent.ctrlKey || keyEvent.altKey || keyEvent.shiftKey) return + actions.opacityDigit(digit) + } + }, + { + id: `opacity-Numpad${digit}`, + keys: `Numpad${digit}`, + run: ({ keyEvent, actions }: KeyboardShortcutRunOptions) => { + if (keyEvent.metaKey || keyEvent.ctrlKey || keyEvent.altKey || keyEvent.shiftKey) return + actions.opacityDigit(digit) + } + } + ]) +} + function shouldIgnoreShortcut(event: KeyboardEvent, options: KeyboardShortcutOptions) { return ( (event.target instanceof Element && event.target.closest('[data-picker-content]') !== null) || @@ -136,7 +159,8 @@ export function registerKeyboardShortcuts(options: KeyboardShortcutOptions) { { id: 'delete', keys: 'Delete', run: ({ actions }) => actions.smartDelete(false) }, { id: 'delete-alt', keys: 'Alt+Delete', run: ({ actions }) => actions.smartDelete(true) }, { id: 'enter', keys: 'Enter', run: ({ actions }) => actions.confirmOrEnterText() }, - { id: 'escape', keys: 'Escape', run: ({ actions }) => actions.escapeOrDeselect() } + { id: 'escape', keys: 'Escape', run: ({ actions }) => actions.escapeOrDeselect() }, + ...opacityBindings() ] const bindings: KeyBindingMap = {} diff --git a/src/app/shell/keyboard/types.ts b/src/app/shell/keyboard/types.ts index b0e311060..d4c3d2a15 100644 --- a/src/app/shell/keyboard/types.ts +++ b/src/app/shell/keyboard/types.ts @@ -12,6 +12,7 @@ export type KeyboardShortcutActions = { toggleUI: () => void toggleAI: () => void exportSelectionPng: () => void + opacityDigit: (digit: string) => void } export type KeyboardShortcutOptions = { diff --git a/src/app/shell/keyboard/use.ts b/src/app/shell/keyboard/use.ts index 08ad1d44f..38abea9c5 100644 --- a/src/app/shell/keyboard/use.ts +++ b/src/app/shell/keyboard/use.ts @@ -9,7 +9,6 @@ import { createKeyboardActions } from '@/app/shell/keyboard/actions' import { bindEditorClipboard } from '@/app/shell/keyboard/clipboard' import { isInputElement } from '@/app/shell/keyboard/focus' import { bindNudgeKeys } from '@/app/shell/keyboard/nudging' -import { bindOpacityKeys } from '@/app/shell/keyboard/opacity' import { registerKeyboardShortcuts } from '@/app/shell/keyboard/registry' import { openFileDialog } from '@/app/shell/menu/use' import { closeTab, createTab, activeTab as activeTabRef } from '@/app/tabs' @@ -22,11 +21,16 @@ export function useKeyboard() { const activeElement = useActiveElement() const inputFocused = computed(() => isInputElement(activeElement.value)) - const actions = createKeyboardActions({ store, activeTab, isMobile, runCommand }) + const actions = createKeyboardActions({ + store, + activeTab, + isMobile, + runCommand, + setOpacityTarget + }) bindEditorClipboard(store) bindNudgeKeys(store) - bindOpacityKeys({ store, setOpacityTarget, runCommand }) registerKeyboardShortcuts({ inputFocused, diff --git a/tests/engine/editor/opacity.test.ts b/tests/engine/editor/opacity.test.ts index 691b338c4..495249af6 100644 --- a/tests/engine/editor/opacity.test.ts +++ b/tests/engine/editor/opacity.test.ts @@ -4,7 +4,7 @@ import { createEditor } from '@open-pencil/core/editor' import { getNodeOrThrow } from '#tests/helpers/assert' -describe('opacity via updateNodeWithUndo + runBatch', () => { +describe('editor.setOpacity', () => { function setup() { const editor = createEditor() const pageId = editor.graph.getPages()[0].id @@ -19,36 +19,39 @@ describe('opacity via updateNodeWithUndo + runBatch', () => { return { editor, rect } } - function setOpacity(editor: ReturnType, opacity: number) { - const targets = editor.getSelectedNodes() - if (targets.length === 0) return - editor.undo.runBatch('Set opacity', () => { - for (const target of targets) { - if (target.opacity === opacity) continue - editor.updateNodeWithUndo(target.id, { opacity }, 'Set opacity') - } - }) - } - test('sets opacity to 50%', () => { const { editor, rect } = setup() - setOpacity(editor, 0.5) + editor.setOpacity(0.5) expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(0.5) }) - test('0 digit sets opacity to 100%', () => { + test('sets opacity to 100% (digit 0)', () => { const { editor, rect } = setup() - setOpacity(editor, 0.5) - setOpacity(editor, 1) + editor.setOpacity(0.5) + editor.setOpacity(1) expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(1) }) + test('clamps opacity above 100%', () => { + const { editor, rect } = setup() + + editor.setOpacity(1.5) + expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(1) + }) + + test('clamps opacity below 0%', () => { + const { editor, rect } = setup() + + editor.setOpacity(-0.3) + expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(0) + }) + test('opacity change is undoable as a single batch entry', () => { const { editor, rect } = setup() - setOpacity(editor, 0.3) + editor.setOpacity(0.3) expect(editor.undo.canUndo).toBe(true) editor.undo.undo() @@ -70,7 +73,7 @@ describe('opacity via updateNodeWithUndo + runBatch', () => { }) editor.select([rect.id, rect2.id]) - setOpacity(editor, 0.7) + editor.setOpacity(0.7) expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(0.7) expect(getNodeOrThrow(editor.graph, rect2.id).opacity).toBe(0.7) @@ -82,8 +85,16 @@ describe('opacity via updateNodeWithUndo + runBatch', () => { test('no-op when opacity already matches', () => { const { editor, rect } = setup() - setOpacity(editor, 1) + editor.setOpacity(1) expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(1) expect(editor.undo.canUndo).toBe(false) }) + + test('no-op with no selection', () => { + const { editor } = setup() + + editor.clearSelection() + editor.setOpacity(0.5) + expect(editor.undo.canUndo).toBe(false) + }) })