Skip to content

Commit

Permalink
[fixed] dragSystem dispatching the same value twice on pointerup
Browse files Browse the repository at this point in the history
Summary: up sends the same value for `pageX` and `pageY` as the final move, so we should ignore x and y in up.

Reviewers: O2 Material Motion, O3 Material JavaScript platform reviewers, #material_motion, featherless

Reviewed By: O2 Material Motion, #material_motion, featherless

Tags: #material_motion

Differential Revision: http://codereview.cc/D3127
  • Loading branch information
appsforartists committed May 2, 2017
1 parent 8ed8ac5 commit 40f2cce
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 5 deletions.
113 changes: 113 additions & 0 deletions packages/core/src/systems/__tests__/dragSystem.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/** @license
* Copyright 2016 - present The Material Motion Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy
* of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

import { expect } from 'chai';

import {
beforeEach,
describe,
it,
} from 'mocha-sugar-free';

import {
stub,
} from 'sinon';

declare function require(name: string);

// chai really doesn't like being imported as an ES2015 module; will be fixed in v4
require('chai').use(
require('sinon-chai')
);

import {
createMockObserver,
} from 'material-motion-testing-utils';

import {
MotionObservable,
} from '../../observables/';

import {
createProperty,
} from '../../properties/';

import {
dragSystem,
} from '../';

describe('dragSystem',
() => {
let drag$;
let state;
let recognitionThreshold;
let downObserver;
let moveObserver;
let upObserver;
let listener;

beforeEach(
() => {
state = createProperty();
recognitionThreshold = createProperty({ initialValue: 16 });
downObserver = createMockObserver();
moveObserver = createMockObserver();
upObserver = createMockObserver();
listener = stub();

drag$ = dragSystem({
down$: new MotionObservable(downObserver.connect),
move$: new MotionObservable(moveObserver.connect),
up$: new MotionObservable(upObserver.connect),
recognitionThreshold: recognitionThreshold,
state: state,
});
drag$.subscribe(listener);
}
);

it('should not repeat the same value for move and up',
() => {
downObserver.next({
type: 'pointerdown',
pageX: 0,
pageY: 0,
});

moveObserver.next({
type: 'pointermove',
pageX: 10,
pageY: 20,
});

upObserver.next({
type: 'pointerup',
pageX: 10,
pageY: 20,
});

expect(listener).to.have.been.calledOnce.and.to.have.been.calledWith({
x: 10,
y: 20,
});
}
);
it('should set state to BEGAN when the recognitionThreshold is crossed');
it('should set state to CHANGED on the move after BEGAN');
it('should set state to ENDED on up, if recognized');
it('should set state to POSSIBLE after ENDED');
}
);
8 changes: 3 additions & 5 deletions packages/core/src/systems/dragSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,17 @@ export function dragSystem({
// This would be a takeWhile if we were using an Observable
// implementation that supported completion.
moveSubscription.unsubscribe();
moveSubscription = undefined;

if (state.read() === GestureRecognitionState.POSSIBLE) {
state.write(GestureRecognitionState.FAILED);
} else {
state.write(GestureRecognitionState.ENDED);
}
}

observer.next(translation);

if (atRest) {
state.write(GestureRecognitionState.POSSIBLE);
moveSubscription = undefined;
} else {
observer.next(translation);
}
}
);
Expand Down

0 comments on commit 40f2cce

Please sign in to comment.