fix(editor): harden opacity shortcuts
- Match numeric keypad input by generated key so NumLock-off navigation remains available - Coalesce buffered opacity updates into one undo interaction - Add end-to-end coverage for digit composition, modifiers, and undo
This commit is contained in:
parent
5bcc44cc91
commit
798a3c2b2c
1
bun.lock
1
bun.lock
|
|
@ -44,7 +44,6 @@
|
|||
"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",
|
||||
|
|
|
|||
|
|
@ -97,7 +97,6 @@
|
|||
"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",
|
||||
|
|
|
|||
|
|
@ -60,7 +60,7 @@ export function createNodeActions(ctx: EditorContext) {
|
|||
ctx.requestRender()
|
||||
}
|
||||
|
||||
function setOpacity(opacity: number) {
|
||||
function setOpacity(opacity: number, coalesceKey?: string) {
|
||||
if (!Number.isFinite(opacity)) return
|
||||
const clamped = Math.max(0, Math.min(1, opacity))
|
||||
const ids = [...ctx.state.selectedIds]
|
||||
|
|
@ -68,11 +68,15 @@ export function createNodeActions(ctx: EditorContext) {
|
|||
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', () => {
|
||||
ctx.undo.runBatch(
|
||||
'Set opacity',
|
||||
() => {
|
||||
for (const target of changed) {
|
||||
updateNodeWithUndo(target.id, { opacity: clamped }, 'Set opacity')
|
||||
}
|
||||
})
|
||||
},
|
||||
coalesceKey
|
||||
)
|
||||
}
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ export interface UndoEntry {
|
|||
label: string
|
||||
forward: () => void
|
||||
inverse: () => void
|
||||
coalesceKey?: string
|
||||
}
|
||||
|
||||
export interface UndoManagerOptions {
|
||||
|
|
@ -11,6 +12,7 @@ export interface UndoManagerOptions {
|
|||
interface UndoBatch {
|
||||
label: string
|
||||
entries: UndoEntry[]
|
||||
coalesceKey?: string
|
||||
}
|
||||
|
||||
const DEFAULT_HISTORY_LIMIT = 200
|
||||
|
|
@ -63,8 +65,8 @@ export class UndoManager {
|
|||
return entry.label
|
||||
}
|
||||
|
||||
beginBatch(label: string): void {
|
||||
this.batches.push({ label, entries: [] })
|
||||
beginBatch(label: string, coalesceKey?: string): void {
|
||||
this.batches.push({ label, entries: [], coalesceKey })
|
||||
}
|
||||
|
||||
commitBatch(): void {
|
||||
|
|
@ -77,8 +79,8 @@ export class UndoManager {
|
|||
else this.pushUndoEntry(entry)
|
||||
}
|
||||
|
||||
runBatch<T>(label: string, fn: () => T): T {
|
||||
this.beginBatch(label)
|
||||
runBatch<T>(label: string, fn: () => T, coalesceKey?: string): T {
|
||||
this.beginBatch(label, coalesceKey)
|
||||
try {
|
||||
const result = fn()
|
||||
this.commitBatch()
|
||||
|
|
@ -129,12 +131,21 @@ export class UndoManager {
|
|||
return {
|
||||
label: batch.label,
|
||||
forward: () => batch.entries.forEach((entry) => entry.forward()),
|
||||
inverse: () => batch.entries.toReversed().forEach((entry) => entry.inverse())
|
||||
inverse: () => batch.entries.toReversed().forEach((entry) => entry.inverse()),
|
||||
coalesceKey: batch.coalesceKey
|
||||
}
|
||||
}
|
||||
|
||||
private pushUndoEntry(entry: UndoEntry): void {
|
||||
const previous = this.undoStack.at(-1)
|
||||
if (entry.coalesceKey && previous?.coalesceKey === entry.coalesceKey) {
|
||||
this.undoStack[this.undoStack.length - 1] = {
|
||||
...entry,
|
||||
inverse: previous.inverse
|
||||
}
|
||||
} else {
|
||||
this.undoStack.push(entry)
|
||||
}
|
||||
this.redoStack = []
|
||||
this.trimUndoStack()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,5 +16,5 @@ export type EditorCommandMapOptions = {
|
|||
messages: CommandMessagesStore
|
||||
otherPages: ComputedRef<Array<{ id: string }>>
|
||||
moveSelectionToPage: (pageId: string) => void
|
||||
getOpacityTarget: () => number
|
||||
getOpacityTarget: () => { value: number; coalesceKey?: string }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -269,7 +269,10 @@ export function createSelectionCommands({
|
|||
return t.value.setOpacity
|
||||
},
|
||||
enabled: capabilities.canSetOpacity,
|
||||
run: () => editor.setOpacity(getOpacityTarget())
|
||||
run: () => {
|
||||
const target = getOpacityTarget()
|
||||
editor.setOpacity(target.value, target.coalesceKey)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -41,9 +41,9 @@ export function useEditorCommands() {
|
|||
editor.moveToPage(pageId)
|
||||
}
|
||||
|
||||
let opacityTarget = 1
|
||||
function setOpacityTarget(value: number) {
|
||||
opacityTarget = value
|
||||
let opacityTarget: { value: number; coalesceKey?: string } = { value: 1 }
|
||||
function setOpacityTarget(value: number, coalesceKey?: string) {
|
||||
opacityTarget = coalesceKey ? { value, coalesceKey } : { value }
|
||||
}
|
||||
|
||||
const commands = createEditorCommandMap({
|
||||
|
|
|
|||
|
|
@ -102,18 +102,31 @@ export function createKeyboardActions({
|
|||
}
|
||||
|
||||
let opacityBuffer = ''
|
||||
let opacitySelectionKey = ''
|
||||
let opacityCoalesceKey = ''
|
||||
let opacityResetTimer: ReturnType<typeof setTimeout> | undefined
|
||||
|
||||
function resetOpacityBuffer() {
|
||||
opacityBuffer = ''
|
||||
opacitySelectionKey = ''
|
||||
opacityCoalesceKey = ''
|
||||
clearTimeout(opacityResetTimer)
|
||||
}
|
||||
|
||||
function opacityDigit(digit: string) {
|
||||
if (store.state.selectedIds.size === 0) return
|
||||
const selectionKey = [...store.state.selectedIds].sort().join('\0')
|
||||
if (selectionKey !== opacitySelectionKey) resetOpacityBuffer()
|
||||
if (!opacityBuffer) {
|
||||
opacitySelectionKey = selectionKey
|
||||
opacityCoalesceKey = crypto.randomUUID()
|
||||
}
|
||||
opacityBuffer += digit
|
||||
if (opacityBuffer.length > 3) opacityBuffer = opacityBuffer.slice(-3)
|
||||
setOpacityTarget(opacityFromBuffer(opacityBuffer))
|
||||
setOpacityTarget(opacityFromBuffer(opacityBuffer), opacityCoalesceKey)
|
||||
runCommand('selection.setOpacity')
|
||||
clearTimeout(opacityResetTimer)
|
||||
opacityResetTimer = setTimeout(() => {
|
||||
opacityBuffer = ''
|
||||
}, 800)
|
||||
opacityResetTimer = setTimeout(resetOpacityBuffer, 800)
|
||||
}
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ type ShortcutDefinition = {
|
|||
id: string
|
||||
keys: string | string[]
|
||||
run: ShortcutAction
|
||||
shouldPreventDefault?: (event: KeyboardEvent) => boolean
|
||||
}
|
||||
|
||||
function commandShortcut(
|
||||
|
|
@ -36,19 +37,16 @@ function commandShortcuts(...commands: EditorCommandId[]): ShortcutDefinition[]
|
|||
})
|
||||
}
|
||||
|
||||
const OPACITY_CODES = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'].flatMap((d) => [
|
||||
`Digit${d}`,
|
||||
`Numpad${d}`
|
||||
])
|
||||
|
||||
function opacityBindings(): ShortcutDefinition[] {
|
||||
return OPACITY_CODES.map((code) => ({
|
||||
id: `opacity-${code}`,
|
||||
keys: code,
|
||||
run: ({ keyEvent, actions }: KeyboardShortcutRunOptions) => {
|
||||
return ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'].map((digit) => ({
|
||||
id: `selection-opacity-${digit}`,
|
||||
keys: digit,
|
||||
run: ({ keyEvent, actions }) => {
|
||||
if (keyEvent.metaKey || keyEvent.ctrlKey || keyEvent.altKey || keyEvent.shiftKey) return
|
||||
actions.opacityDigit(code.slice(-1))
|
||||
}
|
||||
actions.opacityDigit(digit)
|
||||
},
|
||||
shouldPreventDefault: (event) =>
|
||||
!event.metaKey && !event.ctrlKey && !event.altKey && !event.shiftKey
|
||||
}))
|
||||
}
|
||||
|
||||
|
|
@ -161,8 +159,8 @@ export function registerKeyboardShortcuts(options: KeyboardShortcutOptions) {
|
|||
|
||||
for (const shortcut of shortcuts) {
|
||||
bindShortcut(bindings, shortcut.keys, (event) => {
|
||||
event.preventDefault()
|
||||
shortcut.run(runOptions(event))
|
||||
if (shortcut.shouldPreventDefault?.(event) ?? true) event.preventDefault()
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -48,6 +48,15 @@ function getZoom() {
|
|||
})
|
||||
}
|
||||
|
||||
function getSelectedOpacity() {
|
||||
return editor.page.evaluate(() => {
|
||||
const store = window.openPencil?.getStore?.()
|
||||
if (!store) throw new Error('OpenPencil store not initialized')
|
||||
const id = [...store.state.selectedIds][0]
|
||||
return id ? store.graph.getNode(id)?.opacity : undefined
|
||||
})
|
||||
}
|
||||
|
||||
test.describe('tool switching', () => {
|
||||
test('V → SELECT', async () => {
|
||||
await editor.page.keyboard.press('v')
|
||||
|
|
@ -222,6 +231,56 @@ test.describe('duplicate', () => {
|
|||
})
|
||||
})
|
||||
|
||||
test.describe('opacity shortcuts', () => {
|
||||
test.beforeEach(async () => {
|
||||
await editor.canvas.clearCanvas()
|
||||
await editor.canvas.drawRect(100, 100, 60, 60)
|
||||
})
|
||||
|
||||
test('combines digits and undoes them as one interaction', async () => {
|
||||
await editor.page.keyboard.press('2')
|
||||
await editor.page.keyboard.press('8')
|
||||
expect(await getSelectedOpacity()).toBe(0.28)
|
||||
|
||||
await editor.page.keyboard.press('Meta+z')
|
||||
expect(await getSelectedOpacity()).toBe(1)
|
||||
|
||||
await editor.page.keyboard.press('Meta+Shift+z')
|
||||
expect(await getSelectedOpacity()).toBe(0.28)
|
||||
})
|
||||
|
||||
test('maps 0 to 100% and 00 to 0%', async () => {
|
||||
await editor.page.keyboard.press('5')
|
||||
expect(await getSelectedOpacity()).toBe(0.5)
|
||||
|
||||
await editor.canvas.clearCanvas()
|
||||
await editor.canvas.drawRect(100, 100, 60, 60)
|
||||
await editor.page.keyboard.press('0')
|
||||
expect(await getSelectedOpacity()).toBe(1)
|
||||
await editor.page.keyboard.press('0')
|
||||
expect(await getSelectedOpacity()).toBe(0)
|
||||
})
|
||||
|
||||
test('does not consume shifted digits or NumLock-off navigation keys', async () => {
|
||||
await editor.page.keyboard.press('5')
|
||||
await editor.page.keyboard.press('Shift+1')
|
||||
expect(await getSelectedOpacity()).toBe(0.5)
|
||||
|
||||
const prevented = await editor.page.evaluate(() => {
|
||||
const event = new KeyboardEvent('keydown', {
|
||||
key: 'End',
|
||||
code: 'Numpad1',
|
||||
bubbles: true,
|
||||
cancelable: true
|
||||
})
|
||||
window.dispatchEvent(event)
|
||||
return event.defaultPrevented
|
||||
})
|
||||
expect(prevented).toBe(false)
|
||||
expect(await getSelectedOpacity()).toBe(0.5)
|
||||
})
|
||||
})
|
||||
|
||||
test.describe('zoom shortcuts', () => {
|
||||
test('⌘0 zooms to 100%', async () => {
|
||||
await editor.canvas.clearCanvas()
|
||||
|
|
|
|||
|
|
@ -61,6 +61,31 @@ describe('editor.setOpacity', () => {
|
|||
expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(0.3)
|
||||
})
|
||||
|
||||
test('coalesces buffered shortcut updates into one undo entry', () => {
|
||||
const { editor, rect } = setup()
|
||||
|
||||
editor.setOpacity(0.2, 'shortcut-session')
|
||||
editor.setOpacity(0.28, 'shortcut-session')
|
||||
|
||||
editor.undo.undo()
|
||||
expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(1)
|
||||
|
||||
editor.undo.redo()
|
||||
expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(0.28)
|
||||
})
|
||||
|
||||
test('keeps separate shortcut sessions as separate undo entries', () => {
|
||||
const { editor, rect } = setup()
|
||||
|
||||
editor.setOpacity(0.2, 'shortcut-session-1')
|
||||
editor.setOpacity(0.8, 'shortcut-session-2')
|
||||
|
||||
editor.undo.undo()
|
||||
expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(0.2)
|
||||
editor.undo.undo()
|
||||
expect(getNodeOrThrow(editor.graph, rect.id).opacity).toBe(1)
|
||||
})
|
||||
|
||||
test('opacity batch for multiple selections collapses to one undo entry', () => {
|
||||
const { editor, rect } = setup()
|
||||
const pageId = editor.graph.getPages()[0].id
|
||||
|
|
|
|||
Loading…
Reference in a new issue