diff --git a/README.md b/README.md index a17ac4a..43c589d 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,10 @@ to decode. The blade itself looking for `.tl__cue`, a class that has never existed. A panel dropped into the empty column landing nowhere, because tearing down the drag collapsed that column before the drop was worked out. Six panel tabs overflowing their column into a scrollbar that had been switched off, so -two panels could not be opened at all. None of those is reachable from Node, +two panels could not be opened at all. Every tool writing a point into a +layer's automation, because the handler that routes them stepped aside for a +curve lane and the curve editor asks only whether the pen is held — so panning +across an open layer edited the piece. None of those is reachable from Node, none is a type error, and every one of them shipped. Each test names the fault it is for. `test/browser/app.ts` holds what they all @@ -108,11 +111,20 @@ seconds of canvas recorded through `MediaRecorder` in the page — about ninety kilobytes, made in under a second, and no binary in the repository that somebody has to take on trust. +`tool-scope.spec.ts` is the odd one out and worth knowing about: it is a tool +crossed with a surface rather than a tool on its own, because the tools were +right about what they do and wrong about where. Every case is "this tool, on +that surface, does this and nothing else". + Every test here was checked the same way as the ones above, by putting the -fault back and watching it fail, and writing them found two more: the transport -was hiding its last three controls behind a scrollbar it had disabled, and the -frame rate measurement that runs just after a clip loads was undoing a play -started while it ran. +fault back and watching it fail, and writing them found four more: the +transport was hiding its last three controls behind a scrollbar it had +disabled; the frame rate measurement that runs just after a clip loads was +undoing a play started while it ran; the picture could be dragged off its own +frame, because the pan was clamped against the stage box rather than the +video's and the stage carries fourteen pixels of padding; and zooming back out +never quite reached Fit, since 2.56 divided by 1.6 twice is 1.0000000000000002 +rather than 1. What is not tested yet: the parts that reach the audio graph. diff --git a/src/app.ts b/src/app.ts index 12f84ea..c73d6fe 100644 --- a/src/app.ts +++ b/src/app.ts @@ -131,6 +131,41 @@ export function mountApp(root: HTMLElement, options: AudioEngineOptions = {}): ( */ root.appendChild(el('div', { class: 'frame' }, [keepNotice.el, shell])); + /* + * The tool the pointer is holding, written once for the whole screen. + * + * An attribute on the shell rather than a cursor set on each of the dozen + * surfaces underneath, so a tool cannot be half applied: whatever is under + * the pointer, the shape it takes says which tool is held. It sat on the + * timeline until the picture gained a zoom of its own, and a stage that is + * not inside the timeline cannot read an attribute that is. + */ + const writeTool = (state: AppState): void => { + shell.dataset.tool = state.tool; + }; + // Written now as well as on every change: subscribing does not deliver the + // state as it stands, and a shell with no tool on it at all is a shell that + // matches "not the move tool". + writeTool(session.store.state); + session.store.subscribe(writeTool); + + /* + * Alt, watched only so the zoom tool can say which way it will go. + * + * On the window rather than on a panel because a modifier held down before + * the pointer arrives is the common case, and an element only hears about + * keys while it has focus. Both edges are needed: releasing alt somewhere + * else would otherwise leave the cursor promising a zoom out that is no + * longer what a click does. Blur clears it for the same reason -- alt is + * often what took the window away. + */ + const readAlt = (event: KeyboardEvent): void => { + shell.classList.toggle('is-alt', event.altKey); + }; + window.addEventListener('keydown', readAlt); + window.addEventListener('keyup', readAlt); + window.addEventListener('blur', () => shell.classList.remove('is-alt')); + /** * Whether the video is floating, which is the one thing that changes the * layout: with the clip in a window the stage above the lanes is not just @@ -438,16 +473,18 @@ function editKey(soundDesign: SoundDesignSession, event: KeyboardEvent): boolean * Holding shift moves the selected sound instead, so a hit that feels late * can be pulled back without losing your place. * - * The letters are shared, and the record button decides who has them. + * Some letters are shared, and the record button decides who has those. * - * There are thirteen drum pads on the letter keys and an editor wants those - * same letters for its tools: T, H, J, K, L and S were claimed by both. That - * is not a clash to arbitrate key by key, it is two modes -- you are either - * playing something in or you are editing, and never both in the same - * keystroke. Record already said as much on its own tooltip and then did - * nothing at all, so it is what says which. Armed, the letters are drums; - * otherwise they are tools, which is what somebody arriving from an edit - * suite will try first. + * There are thirteen drum pads on the letter keys and an editor wants some of + * the same letters for its tools. Six are claimed twice: T, H, J, K, L and S. + * You are either playing something in or you are editing, never both in the + * same keystroke, and record already said as much on its own tooltip -- so it + * is what settles those six. Armed they are drums; otherwise they are tools, + * which is what somebody arriving from an edit suite will try first. + * + * Only those six, though. C, V, Z and P are not pads, and arming once took + * them anyway: reaching for the blade with record on did nothing at all and + * said nothing about why. A clash is settled where there is one. */ function soundDesignKey( session: Session, @@ -510,15 +547,24 @@ function soundDesignKey( return; } - /* ---- armed: the letters are drums ---- */ - + /* + * Armed: the letters that are drums are drums. The rest are still tools. + * + * This used to swallow every letter while the record button was on, which + * is more than the clash needs. Only six letters are claimed twice -- T, H, + * J, K, L and S -- and C, V, Z and P are not pads at all, so arming meant + * giving up four tools to a conflict they were never in. Reaching for the + * blade with record on did nothing and said nothing. + */ if (session.state.armed) { const pad = PAD_KEYS[lower]; - if (pad && !event.repeat) { - event.preventDefault(); - soundDesign.addCueAtPlayhead({ kind: 'kit', name: pad }); + if (pad) { + if (!event.repeat) { + event.preventDefault(); + soundDesign.addCueAtPlayhead({ kind: 'kit', name: pad }); + } + return; } - return; } /* ---- otherwise: the letters are an editor's ---- */ diff --git a/src/store.ts b/src/store.ts index a389cf6..d709ed7 100644 --- a/src/store.ts +++ b/src/store.ts @@ -115,8 +115,8 @@ export const TOOLS = [ { id: 'move', key: 'V', name: 'Move', job: 'Choose sounds, drag them, drag their edges to change how long they are' }, { id: 'range', key: 'T', name: 'Range', job: 'Drag out a stretch of time. Delete clears every sound inside it' }, { id: 'cut', key: 'C', name: 'Cut', job: 'Click a sound to cut it short at that point' }, - { id: 'hand', key: 'H', name: 'Hand', job: 'Drag the timeline along without moving anything on it' }, - { id: 'zoom', key: 'Z', name: 'Zoom', job: 'Click to go in, alt-click to go out, drag to fill the width with a stretch' }, + { id: 'hand', key: 'H', name: 'Hand', job: 'Drag the timeline along, or the picture once you have gone into it. Nothing on either moves' }, + { id: 'zoom', key: 'Z', name: 'Zoom', job: 'Click to go in, alt-click to go out. On the timeline, drag to fill the width with a stretch; on the picture, Fit gets the whole frame back' }, { id: 'pen', key: 'P', name: 'Pen', job: 'Draw a curve by dragging across an open lane, instead of placing points one at a time' }, ] as const; diff --git a/src/styles/sound-design.css b/src/styles/sound-design.css index 97836c5..428cd20 100644 --- a/src/styles/sound-design.css +++ b/src/styles/sound-design.css @@ -39,6 +39,35 @@ place-items: center; background: var(--stage); padding: 14px; + /* A picture taken in past Fit is larger than the stage by definition, and + without this it spills over the timeline below it. */ + overflow: hidden; +} + +/* + * The magnification, and the way back from it. + * + * Only there while it is saying something: at Fit the picture is whole and a + * chip reading "100% · Fit" is a control that does nothing next to a label + * nobody needs. Bottom right rather than over the middle of the frame, and + * faint until it is pointed at, because it sits on top of the work. + */ +.vstage__fit { + position: absolute; + right: 20px; + bottom: 20px; + z-index: 2; + display: none; + opacity: 0.72; +} + +.vstage.is-zoomed .vstage__fit { + display: inline-flex; +} + +.vstage__fit:hover, +.vstage__fit:focus-visible { + opacity: 1; } .vstage.is-over { @@ -2070,8 +2099,8 @@ * which is both the default arrow somebody expects from a selection tool and * more informative than one shape over everything. */ -.tl[data-tool='range'] .tl__viewport, -.tl[data-tool='range'] .tl__viewport * { +.app[data-tool='range'] .tl__viewport, +.app[data-tool='range'] .tl__viewport * { cursor: text; } @@ -2094,8 +2123,8 @@ * says so. Both of these shipped in that state until an image was built from * the computed value to see whether it actually loaded. */ -.tl[data-tool='cut'] .tl__viewport, -.tl[data-tool='cut'] .tl__viewport * { +.app[data-tool='cut'] .tl__viewport, +.app[data-tool='cut'] .tl__viewport * { cursor: url('data:image/svg+xml,%3Csvg%20xmlns=%22http://www.w3.org/2000/svg%22%20width=%2222%22%20height=%2222%22%20viewBox=%220%200%2022%2022%22%3E%3Cpath%20d=%22M7.4%202h4.2l1.3%208.4H6.1z%22%20fill=%22white%22%20stroke=%22black%22%20stroke-width=%221.2%22%20stroke-linejoin=%22round%22/%3E%3Cpath%20d=%22M9.5%2011.4V20%22%20fill=%22none%22%20stroke=%22black%22%20stroke-width=%222.6%22%20stroke-linecap=%22round%22/%3E%3Cpath%20d=%22M9.5%2011.4V20%22%20fill=%22none%22%20stroke=%22white%22%20stroke-width=%221.2%22%20stroke-linecap=%22round%22/%3E%3C/svg%3E') 9 20, crosshair; } @@ -2110,17 +2139,17 @@ * pointer events is not in the event's path at all, so the press arrives * addressed to the empty lane behind it. */ -.tl[data-tool='cut'] .cue { +.app[data-tool='cut'] .cue { pointer-events: auto; } -.tl[data-tool='hand'] .tl__viewport, -.tl[data-tool='hand'] .tl__viewport * { +.app[data-tool='hand'] .tl__viewport, +.app[data-tool='hand'] .tl__viewport * { cursor: grab; } -.tl[data-tool='zoom'] .tl__viewport, -.tl[data-tool='zoom'] .tl__viewport * { +.app[data-tool='zoom'] .tl__viewport, +.app[data-tool='zoom'] .tl__viewport * { cursor: zoom-in; } @@ -2130,8 +2159,8 @@ * The tool does both jobs off one button, so without this the only way to * find out which one a click will do is to do it. */ -.tl[data-tool='zoom'].is-alt .tl__viewport, -.tl[data-tool='zoom'].is-alt .tl__viewport * { +.app[data-tool='zoom'].is-alt .tl__viewport, +.app[data-tool='zoom'].is-alt .tl__viewport * { cursor: zoom-out; } @@ -2140,6 +2169,60 @@ cursor: grabbing; } +/* + * The picture answers to the same two tools, and says so. + * + * Written against the app rather than the timeline, because the stage is not + * inside the timeline and the tool is a property of the whole screen. The + * other three are deliberately absent: there is nothing on a frame for a + * blade, a time range or a pen to act on, so they keep the plain arrow and + * the press says as much in the status line. + */ +.app[data-tool='hand'] .vstage, +.app[data-tool='hand'] .vstage * { + cursor: grab; +} + +.vstage.is-grabbing, +.vstage.is-grabbing * { + cursor: grabbing; +} + +.app[data-tool='zoom'] .vstage, +.app[data-tool='zoom'] .vstage * { + cursor: zoom-in; +} + +.app[data-tool='zoom'].is-alt .vstage, +.app[data-tool='zoom'].is-alt .vstage * { + cursor: zoom-out; +} + +/* The Fit chip is a button whatever is held, since it is chrome over the + picture rather than part of it. */ +.vstage__fit, +.app[data-tool] .vstage__fit { + cursor: pointer; +} + +/* + * The layer column keeps its own pointers, whatever tool is held. + * + * Renaming a layer, muting it and opening its curves work under every tool, + * the way a track header does in an edit suite -- so the column is not a + * surface the tools apply to. What it must not do is claim otherwise: it was + * showing a text caret over every layer name while the hand or the blade was + * held, promising a rename to a pointer that was about to pan. + */ +.app:not([data-tool='move']) .tl__gutter, +.app:not([data-tool='move']) .tl__gutter .tl__layer-name { + cursor: default; +} + +.app:not([data-tool='move']) .tl__gutter button { + cursor: pointer; +} + /* ---------- context menus ---------- */ @@ -2242,8 +2325,8 @@ cursor: crosshair; } -.tl[data-tool='pen'] .tl__viewport, -.tl[data-tool='pen'] .tl__viewport * { +.app[data-tool='pen'] .tl__viewport, +.app[data-tool='pen'] .tl__viewport * { cursor: url('data:image/svg+xml,%3Csvg%20xmlns=%22http://www.w3.org/2000/svg%22%20width=%2222%22%20height=%2222%22%20viewBox=%220%200%2022%2022%22%3E%3Cpath%20d=%22M15.4%202.2%2019%205.8%208.2%2016.6l-4.6%201%201-4.6z%22%20fill=%22white%22%20stroke=%22black%22%20stroke-width=%221.2%22%20stroke-linejoin=%22round%22/%3E%3Cpath%20d=%22M12.6%205%2016.2%208.6%22%20fill=%22none%22%20stroke=%22black%22%20stroke-width=%221.1%22/%3E%3C/svg%3E') 3 19, crosshair; } diff --git a/src/ui/help.ts b/src/ui/help.ts index d827dc5..a0058cf 100644 --- a/src/ui/help.ts +++ b/src/ui/help.ts @@ -35,10 +35,13 @@ const SECTIONS: readonly Section[] = [ ['Move (V)', 'Choose sounds, drag them along, and drag a sound’s far edge to change how long it lasts. This is what the timeline has always done, and it is what you start on.'], ['Range (T)', 'Drag out a stretch of time across the lanes. Delete clears every sound that starts inside it. Escape lets it go.'], ['Cut (C)', 'Click a sound anywhere along its length to cut it short there. A sound anchored to its end keeps landing on its marker and loses its front instead.'], - ['Hand (H)', 'Drag the timeline along without moving anything on it.'], - ['Zoom (Z)', 'Click to go in, hold alt and click to go out, or drag across a stretch to fill the width with it.'], - ['The letters', 'V, T, C, H and Z change tool, the way they do in an edit suite. They are printed on the buttons so you only have to read them once.'], - ['While record is armed', 'The letters play the drums instead, because thirteen drum pads and five tools want the same keys. Record decides which. Disarm it and the letters are tools again.'], + ['Hand (H)', 'Drag the timeline along, or the picture once you have gone into it. Nothing on either one moves — only your view of it does.'], + ['Zoom (Z)', 'Click to go in, hold alt and click to go out. On the timeline you can also drag across a stretch to fill the width with it. On the picture, a chip in the corner says how far in you are and takes you back to the whole frame.'], + ['Pen (P)', 'Draw a curve by dragging along an open lane, instead of placing points one at a time. Open a layer’s lanes with the A beside its name.'], + ['Where each one works', 'The hand and the zoom work on the timeline and on the picture. The other three are about the timeline: used on the picture they say so rather than doing something surprising. Only Move chooses sounds — that is the whole point of having a tool for it.'], + ['The layer names, down the left', 'These work whatever tool you are holding, the way a track header does in an edit suite: rename, mute, solo and open the curves without putting the tool down.'], + ['The letters', 'V, T, C, H, Z and P change tool, the way they do in an edit suite. They are printed on the buttons so you only have to read them once.'], + ['While record is armed', 'The letters that are also drum pads play the drums instead — T, H, J, K, L and S are wanted by both, and record decides which. The rest are still tools: C, V, Z and P are not pads, so they keep working. Disarm it and all the letters are tools again.'], ], }, { diff --git a/src/ui/sound-design/stage.ts b/src/ui/sound-design/stage.ts index 6f28609..e33244f 100644 --- a/src/ui/sound-design/stage.ts +++ b/src/ui/sound-design/stage.ts @@ -1,18 +1,29 @@ import type { SoundDesignSession } from '../../sound-design-session.ts'; import type { AppState } from '../../store.ts'; -import { button, el } from '../dom.ts'; +import { button, el, setText } from '../dom.ts'; import type { View } from '../view.ts'; export interface VideoStageView extends View { video: HTMLVideoElement; } +/** How far in the picture can be taken, and the smallest step either way. */ +const MOST = 16; +const STEP = 1.6; + /** * The video the sound is being made for. * * The file is read straight off disk, so nothing is uploaded and the clip * never leaves the machine. Its own audio is muted by default and is never * part of an export, because the deliverable is the sound you are making. + * + * The picture can be gone into and moved about, which it could not before: + * it fit the stage and that was the only size it had. That is fine for + * watching and wrong for the work -- finding the exact frame a foot lands on + * means looking closely at a corner of a picture that is a third of the + * window, and a tool called Hand that cannot move the picture is a tool that + * does not do what its name says. */ export function createVideoStage(session: SoundDesignSession): VideoStageView { const video = el('video', { @@ -46,7 +57,181 @@ export function createVideoStage(session: SoundDesignSession): VideoStageView { ]), ]); - const root = el('div', { class: 'vstage' }, [video, drop, picker]); + /** + * How far in, and how far off centre. + * + * One is the picture fitting the stage, which is where it starts and what + * Fit returns it to. The offset is in screen pixels from the middle, which + * is what the transform wants and what the pointer arithmetic is already + * in -- keeping it in picture coordinates would mean converting twice on + * every frame of a drag for no gain. + */ + let scale = 1; + let offset = { x: 0, y: 0 }; + + /** + * The way back, and the only thing on screen that says you are zoomed. + * + * Every editor shows the magnification somewhere, because "why is the + * picture cut off" is otherwise a question with no answer on screen. It is + * not there at all at Fit, where it would be saying nothing. + */ + const fitButton = button( + { + class: 'chip chip--sm vstage__fit', + title: 'Back to the whole picture', + on: { click: () => setView(1, { x: 0, y: 0 }) }, + }, + ['Fit'], + ); + + const root = el('div', { class: 'vstage' }, [video, drop, fitButton, picker]); + + /** + * The size the picture actually covers at rest, letterboxing excluded. + * + * Measured against the video's own box rather than the stage's, which are + * not the same thing: the stage carries fourteen pixels of padding all + * round, so taking the stage's width here overstated the picture by nearly + * thirty pixels and let a drag pull it that far off an edge. `clientWidth` + * rather than a bounding rectangle, because a rectangle is measured after + * the transform and this is the size the transform is applied to. + */ + function fitted(): { width: number; height: number } { + const wide = video.videoWidth || 16; + const tall = video.videoHeight || 9; + const at = Math.min(video.clientWidth / wide, video.clientHeight / tall); + return { width: wide * at, height: tall * at }; + } + + /** + * How far off centre the picture may go before it pulls away from an edge. + * + * Without this a drag can throw the picture off the side and leave you + * looking at the empty stage with no way of knowing which direction it + * went. Zero on an axis that still fits, so a wide clip in a tall stage + * moves sideways only. + */ + function room(at: number): { x: number; y: number } { + const size = fitted(); + return { + x: Math.max(0, (size.width * at - video.clientWidth) / 2), + y: Math.max(0, (size.height * at - video.clientHeight) / 2), + }; + } + + function setView(at: number, to: { x: number; y: number }): void { + /* + * Snapped near the bottom, because 1.6 does not divide back out cleanly. + * + * Two presses in and two alt-presses back out is 2.56 / 1.6 / 1.6, which + * in binary is 1.0000000000000002 rather than 1 -- so the picture stayed + * magnified by a fifteenth of a millionth of a percent, which is to say + * invisibly, and the Fit chip stayed on screen with nothing left to do. + * A tenth of a percent is far below anything anybody can see and far + * above the error. + */ + const wanted = Math.min(MOST, Math.max(1, at)); + scale = wanted < 1.001 ? 1 : wanted; + const edge = room(scale); + offset = { + x: Math.min(edge.x, Math.max(-edge.x, to.x)), + y: Math.min(edge.y, Math.max(-edge.y, to.y)), + }; + // Written rather than left to a class, since it is a continuous value. + video.style.transform = + scale === 1 ? '' : `translate(${offset.x}px, ${offset.y}px) scale(${scale})`; + root.classList.toggle('is-zoomed', scale > 1); + setText(fitButton, `${Math.round(scale * 100)}% · Fit`); + } + + /** Back to the whole picture, for a new clip or a move between homes. */ + const refit = (): void => setView(1, { x: 0, y: 0 }); + + /** + * Go in or out around a point, leaving what is under it where it is. + * + * Zooming around the middle would be simpler and is the thing that makes + * magnification annoying to use: what you were looking at slides away as + * you go in, so every step needs a drag after it to find the subject again. + */ + function zoomAround(clientX: number, clientY: number, to: number): void { + const box = root.getBoundingClientRect(); + const from = scale; + const next = Math.min(MOST, Math.max(1, to)); + if (next === from) return; + if (next === 1) return refit(); + + const px = clientX - (box.x + box.width / 2); + const py = clientY - (box.y + box.height / 2); + const k = next / from; + setView(next, { + x: px - (px - offset.x) * k, + y: py - (py - offset.y) * k, + }); + } + + /** Drag the picture along under a still stage. */ + function panFrom(event: PointerEvent): void { + const fromX = event.clientX; + const fromY = event.clientY; + const was = { ...offset }; + root.classList.add('is-grabbing'); + + const move = (e: PointerEvent): void => { + setView(scale, { x: was.x + (e.clientX - fromX), y: was.y + (e.clientY - fromY) }); + }; + const up = (): void => { + root.classList.remove('is-grabbing'); + window.removeEventListener('pointermove', move); + window.removeEventListener('pointerup', up); + window.removeEventListener('pointercancel', up); + }; + window.addEventListener('pointermove', move); + window.addEventListener('pointerup', up); + window.addEventListener('pointercancel', up); + } + + /** + * The two tools that mean something over a picture, and nothing else. + * + * Cut, Range and Pen are about the timeline; there is nothing on a frame + * for them to act on, so they say so once rather than doing something + * surprising or nothing at all. Move is not here for the same reason it is + * not on the timeline's tool handler: it is the tool that leaves every + * surface behaving as it already did. + */ + function toolPress(event: PointerEvent): void { + if (event.button !== 0) return; + if (!session.store.state.videoReady) return; + const tool = session.store.state.tool; + if (tool === 'move') return; + + event.preventDefault(); + event.stopPropagation(); + + if (tool === 'hand') { + if (scale === 1) { + session.store.set({ status: 'the picture already fits — zoom in first, then drag it about' }); + return; + } + panFrom(event); + return; + } + if (tool === 'zoom') { + const out = event.altKey || event.metaKey || event.ctrlKey; + zoomAround(event.clientX, event.clientY, out ? scale / STEP : scale * STEP); + return; + } + session.store.set({ status: `the ${tool} tool works on the timeline, not on the picture` }); + } + + root.addEventListener('pointerdown', toolPress, true); + + // A stage that changes size changes how far the picture may be moved, and + // a picture left beyond the new edge would be stuck there. + const watch = new ResizeObserver(() => setView(scale, offset)); + watch.observe(root); // Accept a file dropped anywhere on the stage. root.addEventListener('dragover', (event) => { @@ -61,12 +246,30 @@ export function createVideoStage(session: SoundDesignSession): VideoStageView { if (file) void session.loadVideo(file); }); + /** What was loaded and where it was living, to notice either changing. */ + let showing = { name: null as string | null, floating: false }; + return { el: root, video, update(state: AppState) { drop.style.display = state.videoReady ? 'none' : ''; video.style.display = state.videoReady ? '' : 'none'; + + /* + * A new clip, or the picture moving into its own window, starts again + * at Fit. + * + * The transform rides on the video element, and that element is lent to + * the floating window rather than copied into it -- so a magnification + * worked out against the stage would follow it into a box a fifth of + * the size and put the picture somewhere off screen. + */ + const now = { name: state.project.videoName, floating: state.videoWindow }; + if (now.name !== showing.name || now.floating !== showing.floating) { + showing = now; + refit(); + } }, }; } diff --git a/src/ui/sound-design/timeline.ts b/src/ui/sound-design/timeline.ts index 24b4f45..8b0fd45 100644 --- a/src/ui/sound-design/timeline.ts +++ b/src/ui/sound-design/timeline.ts @@ -325,9 +325,26 @@ export function createTimeline( const tool = session.store.state.tool; if (tool === 'move' || event.button !== 0) return; - // Opening and closing a curve lane stays available under every tool: it - // is about what you can see rather than about what you are editing. - if ((event.target as Element | null)?.closest?.('.tl__auto')) return; + /* + * An open curve lane is the pen's surface, and only the pen's. + * + * Every tool used to be let through here, on the reasoning that opening + * and closing a lane is about what you can see rather than what you are + * editing. What that actually did was hand the press to the curve + * editor, which asks only whether the pen is held and treats everything + * else as "add a point and drag it" -- so panning across a layer with + * its curves open wrote a point into the automation, and so did zooming, + * and so did the blade. A tool that quietly edits the piece while doing + * its own job is worse than one that does nothing. + * + * So the lanes are surfaces like any other now: the hand pans over them, + * zoom zooms, and the two tools that draw on a curve -- the pen here, + * Move by falling through above -- are the two that reach it. Lanes are + * opened from the A beside the layer's name, which works whatever is + * held. + */ + const onOpenCurve = (event.target as Element | null)?.closest?.('.tl__auto:not(.is-shut)'); + if (tool === 'pen' && onOpenCurve) return; event.preventDefault(); event.stopPropagation(); @@ -337,9 +354,9 @@ export function createTimeline( if (tool === 'range') return rangeFrom(event); if (tool === 'cut') return cutAt(event); if (tool === 'pen') { - // The pen only has anywhere to draw on an open curve lane, and those - // are excluded above, so arriving here means it was used on the lanes - // themselves. Say where it works rather than swallowing the press. + // Arriving here is the pen used anywhere but an open curve, since that + // is the one case returned above. Say where it works rather than + // swallowing the press. session.store.set({ status: 'the pen draws on a layer’s curve lanes — open them with A beside the layer’s name', }); @@ -466,7 +483,13 @@ export function createTimeline( */ function cutAt(event: PointerEvent): void { const node = (event.target as Element | null)?.closest?.('.cue'); - if (!node) return; + // Said rather than ignored: a blade that does nothing on empty lane looks + // exactly like a blade that is broken, which is how the one that really + // was broken went a whole release without being noticed. + if (!node) { + session.store.set({ status: 'nothing there to cut — the blade shortens a sound you click on' }); + return; + } const id = (node as HTMLElement).dataset.cue; const drawn = id ? cueNodes.get(id) : undefined; if (!id || !drawn) return; @@ -479,24 +502,6 @@ export function createTimeline( viewport.addEventListener('pointerdown', toolPress, true); - /* - * Alt, watched only so the zoom tool can say which way it will go. - * - * On the window rather than the panel because a modifier held down before - * the pointer arrives is the common case, and an element only hears about - * keys while it has focus. Both edges are needed: releasing alt somewhere - * else would otherwise leave the cursor promising a zoom out that is no - * longer what a click does. Blur clears it for the same reason -- alt is - * often what took the window away. - */ - const readAlt = (event: KeyboardEvent): void => { - root.classList.toggle('is-alt', event.altKey); - }; - const clearAlt = (): void => root.classList.remove('is-alt'); - window.addEventListener('keydown', readAlt); - window.addEventListener('keyup', readAlt); - window.addEventListener('blur', clearAlt); - /* ---------- what you can do with the thing under the pointer ---------- */ @@ -1789,15 +1794,6 @@ export function createTimeline( undo.disabled = !session.canUndo; redo.disabled = !session.canRedo; - /* - * The tool decides what the pointer looks like over the whole panel. - * - * A class on the root rather than a cursor set on each of the dozen - * things underneath, so a tool cannot be half applied: whatever is - * under the pointer, the shape it takes says which tool is holding it. - */ - root.dataset.tool = state.tool; - const { range } = state; if (range && state.tool === 'range') { const from = Math.min(range.from, range.to); diff --git a/test/browser/tool-scope.spec.ts b/test/browser/tool-scope.spec.ts new file mode 100644 index 0000000..65374d8 --- /dev/null +++ b/test/browser/tool-scope.spec.ts @@ -0,0 +1,329 @@ +import { expect, test, type Page } from '@playwright/test'; +import { currentTool, cursorOver, loadClip, open, placeSound } from './app.ts'; + +/** + * Where each tool applies, and where it deliberately does not. + * + * The tools were right about what they do and wrong about where. Every one of + * them reached the automation curves, because the handler that routes them + * stepped aside for a curve lane and the curve editor asks only whether the + * pen is held -- so panning across an open layer wrote a point into the + * automation, and so did zooming, and so did the blade. Meanwhile the picture + * was outside the tools altogether: the hand could not move it, because it + * had no size but the one that fit the stage. + * + * Both are about a surface rather than about a tool, which is why they are + * here rather than in tools.spec.ts, and why every case is a tool crossed + * with a place. + */ + +/** Open the first layer's curve lanes, and hand back the one that is open. */ +async function openCurve(page: Page) { + await page.locator('[data-gutter-layer]').first() + .getByRole('button', { name: 'A', exact: true }).click(); + const lane = page.locator('.tl__auto:not(.is-shut)').first(); + await lane.waitFor(); + return lane; +} + +const points = (page: Page) => + page.locator('.tl__auto:not(.is-shut) .tl__auto-point').count(); + +test.describe('the curve lanes', () => { + test.beforeEach(async ({ page }) => { + await open(page); + await loadClip(page); + await placeSound(page, 0.25); + }); + + /* + * The fault, stated as the rule it broke. + * + * Checked with a drag rather than a click because that is what the hand and + * the pen are for, and because a drag is what left a point behind and then + * moved it -- a click alone understates what was happening to the curve. + */ + test('only Move and the pen write points on a curve', async ({ page }) => { + for (const [key, tool, writes] of [ + ['v', 'Move', true], + ['p', 'Pen', true], + ['h', 'Hand', false], + ['z', 'Zoom', false], + ['c', 'Cut', false], + ['t', 'Range', false], + ] as const) { + await page.reload(); + await expect(page.locator('.rail__tool').first()).toBeVisible(); + const lane = await openCurve(page); + await page.keyboard.press(key); + + const before = await points(page); + const box = (await lane.boundingBox())!; + await page.mouse.move(box.x + 40, box.y + box.height / 2); + await page.mouse.down(); + await page.mouse.move(box.x + 180, box.y + box.height / 2 - 8, { steps: 8 }); + await page.mouse.up(); + const after = await points(page); + + if (writes) expect(after, `${tool} draws on a curve`).toBeGreaterThan(before); + else expect(after, `${tool} must not touch the curve`).toBe(before); + } + }); + + test('the hand pans across a curve lane instead of drawing on it', async ({ page }) => { + const lane = await openCurve(page); + for (let i = 0; i < 3; i += 1) { + await page.getByRole('button', { name: '+', exact: true }).click(); + } + await page.keyboard.press('h'); + + const box = (await lane.boundingBox())!; + await page.mouse.move(box.x + 400, box.y + box.height / 2); + await page.mouse.down(); + await page.mouse.move(box.x + 150, box.y + box.height / 2, { steps: 8 }); + await page.mouse.up(); + + const scrolled = await page.locator('.tl__viewport').evaluate((n) => n.scrollLeft); + expect(scrolled, 'the view moved').toBeGreaterThan(0); + expect(await points(page), 'nothing was drawn').toBe(0); + }); + + test('zoom zooms over a curve lane instead of drawing on it', async ({ page }) => { + const lane = await openCurve(page); + await page.keyboard.press('z'); + const wide = () => page.locator('.tl__content').evaluate((n) => n.getBoundingClientRect().width); + + const before = await wide(); + const box = (await lane.boundingBox())!; + await page.mouse.click(box.x + 120, box.y + box.height / 2); + + expect(await wide(), 'it zoomed in').toBeGreaterThan(before); + expect(await points(page), 'nothing was drawn').toBe(0); + }); +}); + +/** + * The picture, which had no size but the one that fit the stage. + * + * A tool called Hand that cannot move the picture does not do what its name + * says, and it could not, because there was nothing to move it to. So the + * zoom is the thing being tested here as much as the pan is. + */ +test.describe('the picture', () => { + const scaleOf = (page: Page) => + page.locator('.vstage__video').evaluate((n) => getComputedStyle(n).transform); + + test.beforeEach(async ({ page }) => { + await open(page); + await loadClip(page); + }); + + test('starts whole, with nothing offering a way back', async ({ page }) => { + expect(await scaleOf(page), 'no transform at Fit').toBe('none'); + await expect(page.locator('.vstage__fit')).toBeHidden(); + }); + + test('the zoom tool goes into the picture, and alt comes back out', async ({ page }) => { + await page.keyboard.press('z'); + const stage = (await page.locator('.vstage').boundingBox())!; + const middle = { x: stage.x + stage.width / 2, y: stage.y + stage.height / 2 }; + + await page.mouse.click(middle.x, middle.y); + await expect(page.locator('.vstage')).toHaveClass(/is-zoomed/); + const inOnce = await scaleOf(page); + expect(inOnce, 'the picture is magnified').not.toBe('none'); + + await page.mouse.click(middle.x, middle.y); + expect(await scaleOf(page), 'a second press goes further in').not.toBe(inOnce); + + // Alt goes back out, and far enough out it is whole again. Held over the + // presses rather than passed to them: `mouse.click` has no modifier of + // its own, so the key has to actually be down. + await page.keyboard.down('Alt'); + await page.mouse.click(middle.x, middle.y); + await page.mouse.click(middle.x, middle.y); + await page.keyboard.up('Alt'); + expect(await scaleOf(page), 'back to the whole picture').toBe('none'); + await expect(page.locator('.vstage')).not.toHaveClass(/is-zoomed/); + }); + + test('the hand moves the picture once there is somewhere to move it', async ({ page }) => { + await page.keyboard.press('z'); + const stage = (await page.locator('.vstage').boundingBox())!; + await page.mouse.click(stage.x + stage.width / 2, stage.y + stage.height / 2); + await page.mouse.click(stage.x + stage.width / 2, stage.y + stage.height / 2); + const before = await scaleOf(page); + + await page.keyboard.press('h'); + await page.mouse.move(stage.x + stage.width * 0.6, stage.y + stage.height / 2); + await page.mouse.down(); + await page.mouse.move(stage.x + stage.width * 0.35, stage.y + stage.height / 2, { steps: 8 }); + await page.mouse.up(); + + expect(await scaleOf(page), 'the picture moved under the hand').not.toBe(before); + // And it is a move, not a zoom: the last two numbers of the matrix are the + // offset, the first and fourth are the scale. + const [a, , , d] = (await scaleOf(page)).replace(/[^0-9.,-]/g, '').split(',').map(Number); + const [a0, , , d0] = before.replace(/[^0-9.,-]/g, '').split(',').map(Number); + expect(a, 'the magnification is unchanged').toBeCloseTo(a0, 3); + expect(d, 'the magnification is unchanged').toBeCloseTo(d0, 3); + }); + + test('Fit puts the whole picture back', async ({ page }) => { + await page.keyboard.press('z'); + const stage = (await page.locator('.vstage').boundingBox())!; + await page.mouse.click(stage.x + stage.width / 2, stage.y + stage.height / 2); + await expect(page.locator('.vstage__fit')).toBeVisible(); + + await page.locator('.vstage__fit').click(); + expect(await scaleOf(page)).toBe('none'); + await expect(page.locator('.vstage__fit')).toBeHidden(); + }); + + /* + * The picture cannot be thrown off the stage. + * + * A pan with no limit leaves you looking at an empty stage with nothing on + * screen saying which way the picture went, and no way back but Fit. + */ + test('the picture cannot be dragged away from the stage', async ({ page }) => { + await page.keyboard.press('z'); + const stage = (await page.locator('.vstage').boundingBox())!; + await page.mouse.click(stage.x + stage.width / 2, stage.y + stage.height / 2); + + await page.keyboard.press('h'); + // Far further than the picture has room for, several times over. + for (let i = 0; i < 3; i += 1) { + await page.mouse.move(stage.x + stage.width - 10, stage.y + stage.height / 2); + await page.mouse.down(); + await page.mouse.move(stage.x - 600, stage.y - 600, { steps: 6 }); + await page.mouse.up(); + } + + /* + * Measured against the video's own box, not the stage's. + * + * The stage has padding, so the picture never reaches its edges and never + * should -- that border is the frame. What must hold is that the + * magnified picture still covers the box it is drawn in, on every side. + * The box is read from `offsetLeft` and `clientWidth`, which a transform + * does not touch; the picture is read from the drawn rectangle, which is + * the same box after it. + */ + const seen = await page.locator('.vstage__video').evaluate((node) => { + const video = node as HTMLVideoElement; + let x = 0; + let y = 0; + for (let up: HTMLElement | null = video; up; up = up.offsetParent as HTMLElement | null) { + x += up.offsetLeft; + y += up.offsetTop; + } + const box = { x, y, width: video.clientWidth, height: video.clientHeight }; + + const drawn = video.getBoundingClientRect(); + const at = Math.min(drawn.width / video.videoWidth, drawn.height / video.videoHeight); + const wide = video.videoWidth * at; + const tall = video.videoHeight * at; + return { + box, + picture: { + left: drawn.x + (drawn.width - wide) / 2, + right: drawn.x + (drawn.width + wide) / 2, + top: drawn.y + (drawn.height - tall) / 2, + bottom: drawn.y + (drawn.height + tall) / 2, + }, + }; + }); + + expect(seen.picture.left, 'it pulled away from the left edge') + .toBeLessThanOrEqual(seen.box.x + 1); + expect(seen.picture.right, 'it pulled away from the right edge') + .toBeGreaterThanOrEqual(seen.box.x + seen.box.width - 1); + expect(seen.picture.top, 'it pulled away from the top edge') + .toBeLessThanOrEqual(seen.box.y + 1); + expect(seen.picture.bottom, 'it pulled away from the bottom edge') + .toBeGreaterThanOrEqual(seen.box.y + seen.box.height - 1); + }); + + test('the tools that have no business on a frame say so', async ({ page }) => { + const stage = (await page.locator('.vstage').boundingBox())!; + for (const [key, tool] of [['c', 'cut'], ['t', 'range'], ['p', 'pen']] as const) { + await page.keyboard.press(key); + await page.mouse.click(stage.x + stage.width / 2, stage.y + stage.height / 2); + await expect(page.locator('.status, [class*="status"]').first()) + .toContainText(`${tool} tool works on the timeline`); + } + expect(await scaleOf(page), 'and none of them moved the picture').toBe('none'); + }); + + test('the picture says which tool is over it', async ({ page }) => { + await page.keyboard.press('h'); + expect(await cursorOver(page, '.vstage__video')).toBe('grab'); + await page.keyboard.press('z'); + expect(await cursorOver(page, '.vstage__video')).toBe('zoom-in'); + await page.keyboard.press('v'); + expect(await cursorOver(page, '.vstage__video'), 'Move leaves it plain').toBe('auto'); + }); +}); + +/** + * The layer column, which the tools deliberately do not apply to. + * + * A track header works whatever tool is held, in this app and in every edit + * suite. What it must not do is claim otherwise, and it was: a text caret over + * every layer name while the hand was held, promising a rename to a pointer + * that was about to pan. + */ +test.describe('the layer column', () => { + test.beforeEach(async ({ page }) => { + await open(page); + await loadClip(page); + }); + + test('does not offer a text caret while a tool is held', async ({ page }) => { + await page.keyboard.press('v'); + expect(await cursorOver(page, '.tl__layer-name'), 'Move can rename').toBe('text'); + + for (const [key, tool] of [['h', 'Hand'], ['c', 'Cut'], ['z', 'Zoom']] as const) { + await page.keyboard.press(key); + expect(await cursorOver(page, '.tl__layer-name'), `${tool} must not promise a rename`) + .toBe('default'); + } + }); + + test('still works while a tool is held', async ({ page }) => { + await page.keyboard.press('h'); + const row = page.locator('[data-gutter-layer]').first(); + await row.getByRole('button', { name: 'A', exact: true }).click(); + await expect(page.locator('.tl__auto:not(.is-shut)')).not.toHaveCount(0); + }); +}); + +/** + * The letters, which the drum pads and the tools both want. + * + * Six are claimed twice and the record button settles those. It used to + * settle all of them, including the four that were never in dispute. + */ +test.describe('the letter keys under record', () => { + test.beforeEach(async ({ page }) => { + await open(page); + await loadClip(page); + await page.getByTitle(/^Record:/).click(); + }); + + test('a letter that is not a pad is still a tool', async ({ page }) => { + for (const [key, name] of [['c', 'Cut tool'], ['z', 'Zoom tool'], ['p', 'Pen tool']] as const) { + await page.keyboard.press(key); + expect(await currentTool(page), `${key} while armed`).toBe(name); + } + }); + + test('a letter that is a pad plays the pad and leaves the tool alone', async ({ page }) => { + await page.keyboard.press('v'); + const before = await page.locator('.cue').count(); + await page.keyboard.press('t'); + await expect(page.locator('.cue')).toHaveCount(before + 1); + expect(await currentTool(page), 'T stayed a drum').toBe('Move tool'); + }); +});