Skip to content
This repository
Browse code

Fix to #3351, dialogs reopening when attempting to close them

  • Loading branch information...
commit 87db1cf5de38108e631e4eb23c7c6dcd56856367 1 parent 7a78822
authored January 16, 2012 gseguin committed January 18, 2012

Showing 1 changed file with 4 additions and 1 deletion. Show diff stats Hide diff stats

  1. 5  js/jquery.mobile.dialog.js
5  js/jquery.mobile.dialog.js
@@ -33,7 +33,10 @@ $.widget( "mobile.dialog", $.mobile.widget, {
33 33
 		// this must be an anonymous function so that select menu dialogs can replace
34 34
 		// the close method. This is a change from previously just defining data-rel=back
35 35
 		// on the button and letting nav handle it
36  
-		headerCloseButton.bind( "vclick", function() {
  36
+		//
  37
+		// Use click rather than vclick in order to prevent the possibility of unintentionally
  38
+		// reopening the dialog if the dialog opening item was directly under the close button.
  39
+		headerCloseButton.bind( "click", function() {
37 40
 			self.close();
38 41
 		});
39 42
 

4 notes on commit 87db1cf

Rob Schieber

Why was this switched? This is causing big problems for us because widgets such as jquery mobile datebox on safari/ios - the controls use vclick and not click to close the dialog. Also, if you read the mobile docs, Jquery mobile has"useFastClick" (vclick) as the default option - doesn't it make more sense for the event to listen for vclick?

Todd Parker

Because of browser quirks, using vclick for navigation like this can result in what seem like double events. Click is safer in this case, even if it's a bit slower.

Rob Schieber

My problem isn't that its slower, my issue with this change is breaks any code which tries to close a dialog by firing a vclick event - the dialog will just hang at the top of the screen and never close - datebox specifically has this issue. It also contradicts the fastclick documentation and creates a very difficult to find bug that only shows on iOS/Safari.

Rob Schieber

Maybe we can find a way to bind to both events so that jqm can maintain compatibility with existing code?

Please sign in to comment.
Something went wrong with that request. Please try again.