Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Panel: Do not close in response to a default-prevented swipe event #6947

Closed
wants to merge 3 commits into from

4 participants

@gabrielschulhof
Collaborator

Fixes gh-6925

Note: This fix requires #6946 even though the automated tests pass, because in the automated tests I trigger swipe manually, with .trigger( "swipeleft" ) instead of calculating it from mouse events using the swipe special event. #6946 ensures that the swipe special event triggers correctly.

@coveralls

Coverage Status

Coverage increased (+0.03%) when pulling 3045191 on 6925-panel-ignores-default-prevented-swipe into cfc1195 on master.

@Ruffio

Shouldn't this PR be closed as it has been merged into master?

@gabrielschulhof
Collaborator

@Ruffio this has not yet been merged into master, AFAIK.

js/widgets/panel.js
((14 lines not shown))
- // on swipe, close the panel
- if ( !!self.options.swipeClose ) {
- if ( self.options.position === "left" ) {
- area.on( "swipeleft.panel", function(/* e */) {
- self.close();
+ // close the panel on swipe if the swipe event's default is not prevented
@arschmitz Owner

capital for first word in comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
js/widgets/panel.js
((14 lines not shown))
- // on swipe, close the panel
- if ( !!self.options.swipeClose ) {
- if ( self.options.position === "left" ) {
- area.on( "swipeleft.panel", function(/* e */) {
- self.close();
+ // close the panel on swipe if the swipe event's default is not prevented
+ if ( !!this.options.swipeClose ) {
@arschmitz Owner

this is a boolean option no need for double inversion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@coveralls

Coverage Status

Coverage increased (+0.11%) when pulling 2ce29a8 on 6925-panel-ignores-default-prevented-swipe into 2dd9cf4 on master.

js/widgets/panel.js
((25 lines not shown))
});
} else {
- area.on( "swiperight.panel", function(/* e */) {
- self.close();
+ this._on( area, {
+ swiperight: "_handleSwipe"
});
}
@arschmitz Owner

This whole if...else can be simplified to

var event = {};

event[ "swipe" + this.options.position ] = "_handleSwipe";
this._on( area, event );
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@arschmitz
Owner

:+1:

@gabrielschulhof gabrielschulhof deleted the 6925-panel-ignores-default-prevented-swipe branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
25 js/widgets/panel.js
@@ -233,21 +233,19 @@ $.widget( "mobile.panel", {
}
},
+ _handleSwipe: function( event ) {
+ if ( !event.isDefaultPrevented() ) {
+ this.close();
+ }
+ },
+
_bindSwipeEvents: function() {
- var self = this,
- area = self._modal ? self.element.add( self._modal ) : self.element;
+ var handler = {};
- // on swipe, close the panel
- if ( !!self.options.swipeClose ) {
- if ( self.options.position === "left" ) {
- area.on( "swipeleft.panel", function(/* e */) {
- self.close();
- });
- } else {
- area.on( "swiperight.panel", function(/* e */) {
- self.close();
- });
- }
+ // Close the panel on swipe if the swipe event's default is not prevented
+ if ( this.options.swipeClose ) {
+ handler[ "swipe" + this.options.position ] = "_handleSwipe";
+ this._on( ( this._modal ? this.element.add( this._modal ) : this.element ), handler );
}
},
@@ -487,7 +485,6 @@ $.widget( "mobile.panel", {
this.element
.removeClass( [ this._getPanelClasses(), o.classes.panelOpen, o.classes.animate ].join( " " ) )
- .off( "swipeleft.panel swiperight.panel" )
.off( "panelbeforeopen" )
.off( "panelhide" )
.off( "keyup.panel" )
View
1  tests/unit/panel/index.html
@@ -55,6 +55,7 @@
<p>Contents of a panel.</p>
</div>
<div data-nstest-role="panel" id="panel-test-dismiss">
+ <input id="dismiss-input"></input>
<p>Contents of a panel.</p>
</div>
<div data-nstest-role="panel" id="panel-test-destroy">
View
43 tests/unit/panel/panel_core.js
@@ -264,7 +264,48 @@
});
- asyncTest( "swipe on dismissable modal closes panel", function() {
+ asyncTest( "swipe on dismissible panel does not close panel if the default is prevented",
+ function() {
+ var panel = $( "#panel-test-dismiss" ),
+ eventNs = ".swipeDoesNotClosePanel",
+ input = $( "#dismiss-input" ).one( "swipeleft", function( event ) {
+ event.preventDefault();
+ });
+
+ expect( 1 );
+
+ $.testHelper.detailedEventCascade([
+ function() {
+ panel.panel( "open" );
+ },
+
+ {
+ panelopen: { src: panel, event: "panelopen" + eventNs + "1" }
+ },
+
+ function() {
+ input.trigger( "swipeleft" );
+ },
+
+ {
+ panelclose: { src: panel, event: "panelclose" + eventNs + "2" }
+ },
+
+ function( result ) {
+ deepEqual( result.panelclose.timedOut, true,
+ "panelclose event did not happen in response to swipe on child input" );
+ panel.panel( "close" );
+ },
+
+ {
+ panelclose: { src: panel, event: "panelclose" + eventNs + "3" }
+ },
+
+ start
+ ]);
+ });
+
+ asyncTest( "swipe on dismissible modal closes panel", function() {
expect( 1 );
Something went wrong with that request. Please try again.