Skip to content

Commit

Permalink
fix: Remove race condition in snap-toggle
Browse files Browse the repository at this point in the history
Disable `tapToClose` feature on `snap-toggle` elements by stifling the mousedown
and mouseup events. You can override this guard with `snap-unsafe="true"`.

See #61
  • Loading branch information
jtrussell committed Jun 19, 2014
1 parent 1634eef commit 909171b
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 3 deletions.
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,16 @@ Note that the default value for `snap-toggle` is `left`. Set it to `right` to to
The directive will honor a `snap-id` attribute when present to explicitly tie it
to a snapper instance.

By default `mouseup` and `mousedown` events are suppressed on elements with this
directive to prevent a race condition with Snap.js's tap-to-close behavior. In
practice this means you will not be able to drag a drawer open by starting your
drag on a toggle button. If you want to keep `mouseup` and `mousedown` events
use the `snap-unsafe` flag:

```html
<button snap-toggle snap-unsafe="true">Toggle Snap</button>
```

### snap-close

Used to easily make a button to close the opened drawer.
Expand Down
2 changes: 1 addition & 1 deletion bower.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "angular-snap-dev",
"version": "1.4.1",
"version": "1.5.1",
"dependencies": {},
"devDependencies": {
"snapjs": "latest",
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "angular-snap-dev",
"version": "1.5.0",
"version": "1.5.1",
"private": true,
"repository": {
"type": "git",
Expand Down
2 changes: 1 addition & 1 deletion src/scripts/constants/snap-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

(function() {
'use strict';
var version = [1, 5, 0]
var version = [1, 5, 1]
, vObj = {
full: version.join('.'),
major: version[0],
Expand Down
35 changes: 35 additions & 0 deletions src/scripts/directives/snap-toggle.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,41 @@ angular.module('snap')
snapId = scope.$eval(snapId);
}

/**
* Stifle mousedown and mouseup events by default
*
* See issue #61
*
* mousedown can create a race condition with the Snap.js `tapToClose`
* setting, the `tapToClose` handler runs first (if drawer is open)
* then our toggle handler runs. Depending on how far along in the
* close animation the drawer is when the toggle handler runs we may
* end up keeping the drawer open (i.e. a quick open/close) or *only*
* performing a double close.
*
* The situation is trickier because we want to allow mouseup events
* to flow through **if** the corresponding mousedown event did not
* target out toggle button... otherwise you could get stuck in a
* drag. We have a naive approach to preventing this... you can still
* get stuck in drag temporarily if you: mouse down on the toggle
* button, then mouse up off screen, then start a drag, then mouse
* down on the toggle button.
*/
if(!attrs.snapUnsafe) {
var downOnMe = false;
element.bind('mousedown', function(event) {
downOnMe = true;
event.stopImmediatePropagation();
});

element.bind('mouseup', function(event) {
if(downOnMe) {
event.stopImmediatePropagation();
}
downOnMe = false;
});
}

element.bind('click', function() {
snapRemote.toggle(snapSide, snapId);
$rootScope.$digest();
Expand Down

0 comments on commit 909171b

Please sign in to comment.