Skip to content

Commit f819461

Browse files
authored
fix: debounce onCropComplete/onCropAreaChange during resize bursts (#653)
* fix: debounce onCropComplete/onCropAreaChange during resize bursts Fixes #309. recomputeCropPosition() always called emitCropData() unconditionally, and computeSizes() now runs on every ResizeObserver tick, so dragging the window/container edge fired onCropComplete and onCropAreaChange dozens of times during a single resize. Resize-triggered computeSizes() calls are now flagged, and their resulting emitCropData() call is debounced (250ms), while all other callers (rotation/aspect/zoom/cropSize changes, media load, etc.) keep firing immediately as before. The crop area itself still updates visually in real time during the resize; only the completion callbacks are debounced. * fix: cancel pending resize debounce timer on immediate crop emit Addresses Copilot review feedback on PR #653: if a resize burst schedules debouncedEmitCropData() and then a non-resize completion (e.g. drag stop, keyboard end) calls emitCropData() immediately, the still-pending resize timer would later fire and call emitCropData() again, causing a duplicate onCropComplete/onCropAreaChange after the user interaction. Clearing the pending timer whenever an immediate emit happens fixes this. * refactor: use named param for isResizeTriggered Per @ValentinH's review, computeSizes and recomputeCropPosition now take { isResizeTriggered } instead of a positional boolean, so call sites are self-documenting.
1 parent 2cd9217 commit f819461

2 files changed

Lines changed: 86 additions & 7 deletions

File tree

cypress/integration/basic_spec.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,48 @@ describe('Basic assertions', function () {
7070
cy.get('#crop-area-y').should('contain', '25')
7171
cy.percySnapshot()
7272
})
73+
74+
it('should debounce onCropComplete during a burst of window resizes', function () {
75+
// let a real frame elapse so the ResizeObserver notification for this
76+
// resize has actually been delivered (mirrors cy.setViewportStable above)
77+
const settle = () =>
78+
cy.window().then(
79+
(win) =>
80+
new Cypress.Promise((resolve) => {
81+
win.requestAnimationFrame(() => win.requestAnimationFrame(() => resolve()))
82+
})
83+
)
84+
const countOnCropComplete = (spy) =>
85+
spy.getCalls().filter((call) => call.args[0] === 'onCropComplete!').length
86+
87+
// let the initial mount settle first so its own onCropComplete call
88+
// doesn't get counted as part of the resize burst below
89+
settle()
90+
cy.window().then((win) => {
91+
cy.spy(win.console, 'log').as('consoleLog')
92+
})
93+
// freeze only the debounce timer itself (not rAF/Date), so ResizeObserver
94+
// notifications are still delivered for real via the settle() waits below,
95+
// but the resulting debounce timeout only fires when we call cy.tick
96+
cy.clock(Date.now(), ['setTimeout', 'clearTimeout'])
97+
98+
// several resizes in a row, simulating dragging the window edge
99+
cy.viewport(700, 600)
100+
settle()
101+
cy.viewport(800, 650)
102+
settle()
103+
cy.viewport(900, 700)
104+
settle()
105+
106+
// right after the burst, onCropComplete should still be debounced (not fired yet)
107+
cy.get('@consoleLog').should((spy) => {
108+
expect(countOnCropComplete(spy)).to.eq(0)
109+
})
110+
111+
// once the debounce window elapses, it should have fired exactly once for the whole burst
112+
cy.tick(260)
113+
cy.get('@consoleLog').should((spy) => {
114+
expect(countOnCropComplete(spy)).to.eq(1)
115+
})
116+
})
73117
})

src/Cropper.tsx

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ import {
1515
} from './helpers'
1616
import cssStyles from './styles.css?raw'
1717

18+
// how long to wait after the last resize-triggered size recompute before
19+
// firing onCropComplete/onCropAreaChange, so a window/container resize
20+
// doesn't spam consumers with callbacks on every frame
21+
const RESIZE_EMIT_DEBOUNCE_TIME = 250
22+
1823
export type CropperProps = {
1924
image?: string
2025
video?: string | VideoSrc[]
@@ -123,6 +128,7 @@ class Cropper extends React.Component<CropperProps, State> {
123128
rafDragTimeout: number | null = null
124129
rafPinchTimeout: number | null = null
125130
wheelTimer: number | null = null
131+
resizeEmitTimer: number | null = null
126132
currentDoc: Document | null = typeof document !== 'undefined' ? document : null
127133
currentWindow: Window | null = typeof window !== 'undefined' ? window : null
128134
resizeObserver: ResizeObserver | null = null
@@ -148,7 +154,7 @@ class Cropper extends React.Component<CropperProps, State> {
148154
this.initResizeObserver()
149155
// only add window resize listener if ResizeObserver is not supported. Otherwise, it would be redundant
150156
if (typeof window.ResizeObserver === 'undefined') {
151-
this.currentWindow.addEventListener('resize', this.computeSizes)
157+
this.currentWindow.addEventListener('resize', this.onWindowResize)
152158
}
153159
this.props.zoomWithScroll &&
154160
this.containerRef.addEventListener('wheel', this.onWheel, { passive: false })
@@ -189,9 +195,12 @@ class Cropper extends React.Component<CropperProps, State> {
189195
componentWillUnmount() {
190196
if (!this.currentDoc || !this.currentWindow) return
191197
if (typeof window.ResizeObserver === 'undefined') {
192-
this.currentWindow.removeEventListener('resize', this.computeSizes)
198+
this.currentWindow.removeEventListener('resize', this.onWindowResize)
193199
}
194200
this.resizeObserver?.disconnect()
201+
if (this.resizeEmitTimer) {
202+
clearTimeout(this.resizeEmitTimer)
203+
}
195204
if (this.containerRef) {
196205
this.containerRef.removeEventListener('gesturestart', this.preventZoomSafari)
197206
}
@@ -250,11 +259,15 @@ class Cropper extends React.Component<CropperProps, State> {
250259
isFirstResize = false // observe() is called on mount, we don't want to trigger a recompute on mount
251260
return
252261
}
253-
this.computeSizes()
262+
this.computeSizes({ isResizeTriggered: true })
254263
})
255264
this.resizeObserver.observe(this.containerRef)
256265
}
257266

267+
onWindowResize = () => {
268+
this.computeSizes({ isResizeTriggered: true })
269+
}
270+
258271
// this is to prevent Safari on iOS >= 10 to zoom the page
259272
preventZoomSafari = (e: Event) => e.preventDefault()
260273

@@ -348,7 +361,7 @@ class Cropper extends React.Component<CropperProps, State> {
348361
return this.props.objectFit
349362
}
350363

351-
computeSizes = () => {
364+
computeSizes = ({ isResizeTriggered = false }: { isResizeTriggered?: boolean } = {}) => {
352365
const mediaRef = this.imageRef.current || this.videoRef.current
353366

354367
if (mediaRef && this.containerRef) {
@@ -435,7 +448,7 @@ class Cropper extends React.Component<CropperProps, State> {
435448
this.props.onCropSizeChange && this.props.onCropSizeChange(cropSize)
436449
}
437450

438-
this.setState({ cropSize }, this.recomputeCropPosition)
451+
this.setState({ cropSize }, () => this.recomputeCropPosition({ isResizeTriggered }))
439452

440453
// pass crop size to parent
441454
if (this.props.setCropSize) {
@@ -705,6 +718,11 @@ class Cropper extends React.Component<CropperProps, State> {
705718
}
706719

707720
emitCropData = () => {
721+
if (this.resizeEmitTimer) {
722+
clearTimeout(this.resizeEmitTimer)
723+
this.resizeEmitTimer = null
724+
}
725+
708726
const cropData = this.getCropData()
709727
if (!cropData) return
710728

@@ -728,7 +746,7 @@ class Cropper extends React.Component<CropperProps, State> {
728746
}
729747
}
730748

731-
recomputeCropPosition = () => {
749+
recomputeCropPosition = ({ isResizeTriggered = false }: { isResizeTriggered?: boolean } = {}) => {
732750
if (!this.state.cropSize) return
733751

734752
let adjustedCrop = this.props.crop
@@ -763,7 +781,24 @@ class Cropper extends React.Component<CropperProps, State> {
763781
this.previousCropSize = this.state.cropSize
764782

765783
this.props.onCropChange(newPosition)
766-
this.emitCropData()
784+
if (isResizeTriggered) {
785+
this.debouncedEmitCropData()
786+
} else {
787+
this.emitCropData()
788+
}
789+
}
790+
791+
// Same as emitCropData but debounced, so that a burst of resize-triggered
792+
// recomputations (e.g. dragging the window edge) only fires
793+
// onCropComplete/onCropAreaChange once, after the resize settles.
794+
debouncedEmitCropData = () => {
795+
if (!this.currentWindow) return
796+
if (this.resizeEmitTimer) {
797+
clearTimeout(this.resizeEmitTimer)
798+
}
799+
this.resizeEmitTimer = this.currentWindow.setTimeout(() => {
800+
this.emitCropData()
801+
}, RESIZE_EMIT_DEBOUNCE_TIME)
767802
}
768803

769804
onKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {

0 commit comments

Comments
 (0)