Work around snap.js state mess

Snap.js is not robust enough to prevent multiple calls to open() or
close(), so we added more checks to prevent these happening even when an
animation is in progress.

Because if we let this through, snap.js will not notice that the
animation is already done (or a duplicate animation was started),
so the "transitionend" event will not fire a second time.

During an animation, snap.js sets up a setInterval() that hogs the CPU
during the animation. Since a transition doesn't always end due to the
above conditions, that CPU hogging would stay forever.

This is the best workaround so far, it seems it's not possible to cancel
that interval from the outside nor to trigger events to make it do so.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>

core/js/tests/specs/coreSpec.js

@@ -903,11 +903,15 @@ describe('Core base tests', function() {
 
 		beforeEach(function() {
 			snapConstructorStub = sinon.stub(window, 'Snap');
-
 			snapperStub = {};
+
 			snapperStub.enable = sinon.stub();
 			snapperStub.disable = sinon.stub();
 			snapperStub.close = sinon.stub();
+			snapperStub.on = sinon.stub();
+			snapperStub.state = sinon.stub().returns({
+				state: sinon.stub()
+			});
 
 			snapConstructorStub.returns(snapperStub);
 

core/src/init.js

@@ -217,18 +217,74 @@ export const initCore = () => {
 
 		$('#app-content').prepend('<div id="app-navigation-toggle" class="icon-menu" style="display:none" tabindex="0"></div>')
 
-		const toggleSnapperOnButton = () => {
+		// keep track whether snapper is currently animating, and
-			if (snapper.state().state === 'left') {
+		// prevent to call open or close while that is the case
-				snapper.close()
+		// to avoid duplicating events (snap.js doesn't check this)
-			} else {
+		let animating = false
-				snapper.open('left')
+		snapper.on('animating', () => {
+			// we need this because the trigger button
+			// is also implicitly wired to close by snapper
+			animating = true
+		})
+		snapper.on('animated', () => {
+			animating = false
+		})
+		snapper.on('start', () => {
+			// we need this because dragging triggers that
+			animating = true
+		})
+		snapper.on('end', () => {
+			// we need this because dragging stop triggers that
+			animating = false
+		})
+
+		// These are necessary because calling open or close
+		// on snapper during an animation makes it trigger an
+		// unfinishable animation, which itself will continue
+		// triggering animating events and cause high CPU load,
+		//
+		// Ref https://github.com/jakiestfu/Snap.js/issues/216
+		const oldSnapperOpen = snapper.open
+		const oldSnapperClose = snapper.close
+		const _snapperOpen = () => {
+			if (animating || snapper.state().state !== 'closed') {
+				return
+			}
+			oldSnapperOpen('left')
+		}
+
+		const _snapperClose = () => {
+			if (animating || snapper.state().state === 'closed') {
+				return
+			}
+			oldSnapperClose()
+		}
+
+		// Needs to be deferred to properly catch in-between
+		// events that snap.js is triggering after dragging.
+		//
+		// Skipped when running unit tests as we are not testing
+		// the snap.js workarounds...
+		if (!window.TESTING) {
+			snapper.open = () => {
+				_.defer(_snapperOpen)
+			}
+			snapper.close = () => {
+				_.defer(_snapperClose)
 			}
 		}
 
-		$('#app-navigation-toggle').click(toggleSnapperOnButton)
+		$('#app-navigation-toggle').click((e) => {
+			// close is implicit in the button by snap.js
+			if (snapper.state().state !== 'left') {
+				snapper.open()
+			}
+		})
 		$('#app-navigation-toggle').keypress(e => {
-			if (e.which === 13) {
+			if (snapper.state().state === 'left') {
-				toggleSnapperOnButton()
+				snapper.close()
+			} else {
+				snapper.open()
 			}
 		})