Skip to content

Commit 101d7f7

Browse files
committed
refactor: extract resolveScrollAction to reduce onTouchMove complexity
Separate scroll dispatch decision (pure) from side effects: - resolveScrollAction: pure function deciding what sequence to send - drainScrollDelta: loop that dispatches resolved actions Drops onTouchMove cognitive complexity from 34 to under 25. Also adds demo/ to biome ignore (build output, already gitignored).
1 parent 4e43f8e commit 101d7f7

3 files changed

Lines changed: 135 additions & 48 deletions

File tree

biome.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,6 @@
4646
}
4747
],
4848
"files": {
49-
"ignore": ["dist/", "node_modules/", "test-results/"]
49+
"ignore": ["dist/", "node_modules/", "test-results/", "demo/"]
5050
}
5151
}

src/gestures/scroll.ts

Lines changed: 72 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -61,28 +61,81 @@ export function touchToCell(
6161
return { x, y }
6262
}
6363

64+
/** Result of deciding what scroll action to take */
65+
type ScrollAction =
66+
| { readonly type: 'send'; readonly seq: string; readonly newWheelAt: number }
67+
| { readonly type: 'skip' }
68+
69+
/** Pure decision: given a scroll direction and strategy, return the action to take */
70+
export function resolveScrollAction(
71+
dir: 'up' | 'down',
72+
strategy: ScrollConfig['strategy'],
73+
cell: { x: number; y: number },
74+
lastWheelAt: number,
75+
now: number,
76+
wheelIntervalMs: number,
77+
): ScrollAction {
78+
if (strategy === 'keys') {
79+
return { type: 'send', seq: pageSeq(dir), newWheelAt: lastWheelAt }
80+
}
81+
if (now - lastWheelAt < wheelIntervalMs) {
82+
return { type: 'skip' }
83+
}
84+
return { type: 'send', seq: scrollSeq(dir, cell.x, cell.y), newWheelAt: now }
85+
}
86+
87+
/** Mutable scroll gesture state */
88+
interface ScrollState {
89+
startY: number
90+
lastY: number
91+
accDelta: number
92+
lastWheelAt: number
93+
}
94+
95+
/** Drain accumulated delta, dispatching scroll actions until exhausted or rate-limited */
96+
function drainScrollDelta(
97+
state: ScrollState,
98+
touch: Touch,
99+
screen: HTMLElement,
100+
term: XTerminal,
101+
config: ScrollConfig,
102+
): void {
103+
while (Math.abs(state.accDelta) >= config.sensitivity) {
104+
const dir = state.accDelta < 0 ? 'down' : 'up'
105+
const cell = touchToCell(touch, screen, term)
106+
const action = resolveScrollAction(
107+
dir,
108+
config.strategy,
109+
cell,
110+
state.lastWheelAt,
111+
Date.now(),
112+
config.wheelIntervalMs,
113+
)
114+
if (action.type === 'skip') break
115+
sendData(term, action.seq)
116+
state.lastWheelAt = action.newWheelAt
117+
state.accDelta -= (state.accDelta < 0 ? -1 : 1) * config.sensitivity
118+
}
119+
}
120+
64121
/** Attach single-finger vertical scroll to the xterm screen */
65122
export function attachScrollGesture(
66123
term: XTerminal,
67124
config: ScrollConfig,
68125
lock: GestureLock,
69126
isDrawerOpen: () => boolean,
70127
): void {
71-
let startY = 0
72-
let lastY = 0
73-
let accDelta = 0
74-
let lastWheelAt = 0
128+
const state: ScrollState = { startY: 0, lastY: 0, accDelta: 0, lastWheelAt: 0 }
75129
let screenEl: HTMLElement | null = null
76130

77131
function onTouchStart(e: Event): void {
78132
if (!(e instanceof TouchEvent)) return
79-
if (e.touches.length === 1) {
80-
const t = e.touches[0]
81-
if (!t) return
82-
startY = t.clientY
83-
lastY = t.clientY
84-
accDelta = 0
85-
}
133+
if (e.touches.length !== 1) return
134+
const t = e.touches[0]
135+
if (!t) return
136+
state.startY = t.clientY
137+
state.lastY = t.clientY
138+
state.accDelta = 0
86139
}
87140

88141
function onTouchMove(e: Event): void {
@@ -92,7 +145,7 @@ export function attachScrollGesture(
92145
if (!t) return
93146

94147
const y = t.clientY
95-
const totalDy = y - startY
148+
const totalDy = y - state.startY
96149

97150
// Try to claim lock if unclaimed
98151
if (lock.current === 'none' && Math.abs(totalDy) > config.sensitivity) {
@@ -104,36 +157,16 @@ export function attachScrollGesture(
104157

105158
e.preventDefault()
106159

107-
const moveDy = y - lastY
108-
lastY = y
109-
accDelta += moveDy
160+
state.accDelta += y - state.lastY
161+
state.lastY = y
110162

111-
// Send one scroll action per sensitivity-worth of pixels
112-
while (Math.abs(accDelta) >= config.sensitivity) {
113-
const dir = accDelta < 0 ? 'down' : 'up'
114-
115-
if (config.strategy === 'keys') {
116-
sendData(term, pageSeq(dir))
117-
} else {
118-
const now = Date.now()
119-
if (now - lastWheelAt < config.wheelIntervalMs) break
120-
lastWheelAt = now
121-
122-
const screen = screenEl
123-
if (!screen) break
124-
const { x, y: row } = touchToCell(t, screen, term)
125-
sendData(term, scrollSeq(dir, x, row))
126-
}
127-
128-
accDelta -= (accDelta < 0 ? -1 : 1) * config.sensitivity
129-
}
163+
const screen = screenEl
164+
if (screen) drainScrollDelta(state, t, screen, term, config)
130165
}
131166

132167
function onTouchEnd(e: Event): void {
133168
if (!(e instanceof TouchEvent)) return
134-
if (lock.current === 'scroll') {
135-
resetLock(lock)
136-
}
169+
if (lock.current === 'scroll') resetLock(lock)
137170
}
138171

139172
function attach(): void {
@@ -144,12 +177,8 @@ export function attachScrollGesture(
144177
}
145178

146179
screenEl = screen
147-
screen.addEventListener('touchstart', onTouchStart, {
148-
passive: true,
149-
})
150-
screen.addEventListener('touchmove', onTouchMove, {
151-
passive: false,
152-
})
180+
screen.addEventListener('touchstart', onTouchStart, { passive: true })
181+
screen.addEventListener('touchmove', onTouchMove, { passive: false })
153182
screen.addEventListener('touchend', onTouchEnd, { passive: true })
154183
screen.addEventListener('touchcancel', onTouchEnd, { passive: true })
155184
}

tests/gestures.test.ts

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import { describe, expect, test } from 'vitest'
22
import { createGestureLock, resetLock, tryLock } from '../src/gestures/lock'
33
import { clampFontSize, touchDistance } from '../src/gestures/pinch'
4-
import { averageY, pageSeq, scrollSeq, terminalGrid, touchToCell } from '../src/gestures/scroll'
4+
import {
5+
averageY,
6+
pageSeq,
7+
resolveScrollAction,
8+
scrollSeq,
9+
terminalGrid,
10+
touchToCell,
11+
} from '../src/gestures/scroll'
512
import { isValidSwipe } from '../src/gestures/swipe'
613
import { mockTerminal } from './fixtures'
714

@@ -212,16 +219,16 @@ describe('terminalGrid', () => {
212219
expect(terminalGrid(rect, term)).toEqual({ cols: 80, rows: 24 })
213220
})
214221

215-
test('uses char measure element when term cols/rows unavailable', () => {
222+
test('falls back to 80x24 when char measure element has zero dimensions', () => {
223+
// happy-dom doesn't compute real layout so getBoundingClientRect returns zeros,
224+
// which exercises the fallback path rather than the measure element path
216225
const term = mockTerminal()
217226
const rect = { width: 800, height: 480 } as DOMRect
218227
const measure = document.createElement('span')
219228
measure.className = 'xterm-char-measure-element'
220229
measure.style.width = '10px'
221230
measure.style.height = '20px'
222231
document.body.appendChild(measure)
223-
224-
// happy-dom may not compute real layout, so this tests the fallback path
225232
const result = terminalGrid(rect, term)
226233
expect(result.cols).toBeGreaterThan(0)
227234
expect(result.rows).toBeGreaterThan(0)
@@ -283,3 +290,54 @@ describe('touchToCell', () => {
283290
expect(touchToCell(makeTouch(-100, -100), screen, term)).toEqual({ x: 1, y: 1 })
284291
})
285292
})
293+
294+
describe('resolveScrollAction', () => {
295+
test('keys strategy returns pageSeq for up', () => {
296+
const action = resolveScrollAction('up', 'keys', { x: 1, y: 1 }, 0, 1000, 50)
297+
expect(action).toEqual({ type: 'send', seq: '\x1b[5~', newWheelAt: 0 })
298+
})
299+
300+
test('keys strategy returns pageSeq for down', () => {
301+
const action = resolveScrollAction('down', 'keys', { x: 1, y: 1 }, 0, 1000, 50)
302+
expect(action).toEqual({ type: 'send', seq: '\x1b[6~', newWheelAt: 0 })
303+
})
304+
305+
test('keys strategy preserves lastWheelAt unchanged', () => {
306+
const action = resolveScrollAction('up', 'keys', { x: 1, y: 1 }, 500, 1000, 50)
307+
expect(action).toEqual({ type: 'send', seq: '\x1b[5~', newWheelAt: 500 })
308+
})
309+
310+
test('wheel strategy returns scrollSeq when interval elapsed', () => {
311+
const action = resolveScrollAction('up', 'wheel', { x: 5, y: 10 }, 0, 1000, 50)
312+
expect(action).toEqual({
313+
type: 'send',
314+
seq: scrollSeq('up', 5, 10),
315+
newWheelAt: 1000,
316+
})
317+
})
318+
319+
test('wheel strategy skips when rate limited', () => {
320+
// lastWheelAt=990, now=1000, interval=50 → 10ms < 50ms → skip
321+
const action = resolveScrollAction('up', 'wheel', { x: 5, y: 10 }, 990, 1000, 50)
322+
expect(action).toEqual({ type: 'skip' })
323+
})
324+
325+
test('wheel strategy sends when exactly at interval boundary', () => {
326+
// lastWheelAt=950, now=1000, interval=50 → 50ms >= 50ms → send
327+
const action = resolveScrollAction('up', 'wheel', { x: 1, y: 1 }, 950, 1000, 50)
328+
expect(action).toEqual({
329+
type: 'send',
330+
seq: scrollSeq('up', 1, 1),
331+
newWheelAt: 1000,
332+
})
333+
})
334+
335+
test('wheel strategy down returns correct sequence', () => {
336+
const action = resolveScrollAction('down', 'wheel', { x: 3, y: 7 }, 0, 1000, 50)
337+
expect(action).toEqual({
338+
type: 'send',
339+
seq: scrollSeq('down', 3, 7),
340+
newWheelAt: 1000,
341+
})
342+
})
343+
})

0 commit comments

Comments
 (0)