From 4792d5b7afe163446b4ce2ed2e49f4d78f62f9c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Scott=20Gonz=C3=A1lez?= <scott.gonzalez@gmail.com>
Date: Thu, 17 Nov 2011 19:48:04 -0500
Subject: [PATCH] Draggable: Cleanup.

---
 ui/jquery.ui.draggable.js | 106 ++++++++++++--------------------------
 1 file changed, 32 insertions(+), 74 deletions(-)

diff --git a/ui/jquery.ui.draggable.js b/ui/jquery.ui.draggable.js
index 180f3038f..d4e98a617 100644
--- a/ui/jquery.ui.draggable.js
+++ b/ui/jquery.ui.draggable.js
@@ -18,10 +18,10 @@ $.widget( "ui.draggable", {
 	widgetEventPrefix: "drag",
 
 	options: {
-		helper: false,
+		helper: null,
+		// TODO: remove scroll options
 		scrollSensitivity: 20,
-		scrollSpeed: 20,
-		iframeFix: false
+		scrollSpeed: 20
 	},
 
 	// dragEl: element being dragged (original or helper)
@@ -34,37 +34,28 @@ $.widget( "ui.draggable", {
 	// overflowOffset: offset of scroll parent
 	// overflow: object containing width and height keys of scroll parent
 
+	// TODO: move next to _unblockFrames()
 	_blockFrames: function() {
+		var body = this.document[0].body;
 
-		var iframes = $('iframe'),
-			widget = this;
+		this.iframeBlocks = this.document.find( "iframe" ).map(function() {
+			var iframe = $( this ),
+				iframeOffset = iframe.offset();
 
-		this.iframeBlocks = $('');
-
-		iframes.each( function() {
-
-			var iframe = $(this),
-				width = iframe.outerWidth(),
-				height = iframe.outerHeight(),
-				iframeOffset = iframe.offset(),
-				block = $('<div />');
-
-		  block.css({
-				position: 'absolute',
-				width: width+'px',
-				height: height+'px',
-				top: iframeOffset.top+'px',
-				left: iframeOffset.left+'px'
-			})
-			.appendTo( widget.document[0].body );
-
-			widget.iframeBlocks = widget.iframeBlocks.add( block );
-			
+			return $( "<div>" )
+				.css({
+					position: "absolute",
+					width: iframe.outerWidth(),
+					height: iframe.outerHeight(),
+					top: iframeOffset.top,
+					left: iframeOffset.left
+				})
+				.appendTo( body )[0];
 		});
-		
 	},
 
 	_create: function() {
+		// TODO: move to drag start in case DOM changes
 		this.scrollParent = this.element.scrollParent();
 
 		// Static position elements can't be moved with top/left
@@ -128,10 +119,7 @@ $.widget( "ui.draggable", {
 			else if ( event.pageX < ( scrollLeft + this.options.scrollSensitivity ) ) {
 				this.scrollParent.scrollLeft( scrollLeft - this.options.scrollSpeed );
 			}
-
 		} else {
-
-
 			// Handle vertical scrolling
 			if ( ( event.pageY + this.options.scrollSensitivity ) > ( this.overflow.height + this.overflowOffset.top ) ) {
 				this.scrollParent.scrollTop( scrollTop + this.options.scrollSpeed );
@@ -147,14 +135,11 @@ $.widget( "ui.draggable", {
 			else if ( ( event.pageX - this.options.scrollSensitivity ) < this.overflowOffset.left ) {
 				this.scrollParent.scrollLeft( scrollLeft - this.options.scrollSpeed );
 			}
-
-
 		}
-
 	},
 
 	_mouseDown: function( event ) {
-		var newLeft, newTop, allowed;
+		var newLeft, newTop;
 
 		// Prevent text selection, among other things
 		event.preventDefault();
@@ -162,10 +147,6 @@ $.widget( "ui.draggable", {
 		// The actual dragging element, should always be a jQuery object
 		this.dragEl = this.element;
 
-		if ( this.options.iframeFix === true ) {
-			this._blockFrames();
-		}
-
 		// Helper required
 		if ( this.options.helper ) {
 			// clone
@@ -217,14 +198,13 @@ $.widget( "ui.draggable", {
 
 		this._preparePosition( event );
 
-		allowed = this._trigger( "start", event, this._uiHash() );
-
 		// If user stops propagation, leave helper there ( if there's one ), disallow any CSS changes
-		if ( allowed !== true ) {
+		if ( this._trigger( "start", event, this._uiHash() ) === false ) {
 			this.document.unbind( "." + this.widgetName );
 			return;
 		}
 
+		this._blockFrames();
 		this._setCss( event );
 
 		this._bind( this.document, {
@@ -234,15 +214,12 @@ $.widget( "ui.draggable", {
 	},
 
 	_mouseMove: function( event ) {
-		var newLeft, newTop, allowed;
+		var newLeft, newTop;
 
 		this._preparePosition( event );
 
-		allowed = this._trigger( "drag", event, this._uiHash() );
-
-
 		// If user stops propagation, leave helper there ( if there's one ), disallow any CSS changes
-		if ( allowed !== true ) {
+		if ( this._trigger( "drag", event, this._uiHash() ) === false ) {
 			this.document.unbind( "." + this.widgetName );
 			return;
 		}
@@ -254,30 +231,18 @@ $.widget( "ui.draggable", {
 	},
 
 	_mouseUp: function( event ) {
-
-		var allowed;
-
 		this._preparePosition( event );
 
-		allowed = this._trigger( "stop", event, this._uiHash() );
-
 		// If user stops propagation, leave helper there, disallow any CSS changes
-		if ( allowed === true ) {
-
+		if ( this._trigger( "stop", event, this._uiHash() ) === false ) {
 			this._setCss( event );
-
 			if ( this.options.helper ) {
 				this.dragEl.remove();
 			}
-
 		}
 
 		this.document.unbind( "." + this.widgetName );
-
-		if ( this.options.iframeFix === true ) {
-			this._unblockFrames();
-		}
-
+		this._unblockFrames();
 	},
 
 	// Uses event to determine new position of draggable, before any override from callbacks
@@ -298,7 +263,7 @@ $.widget( "ui.draggable", {
 			left: newLeft,
 			top: newTop
 		};
-		
+
 		// Refresh offset cache with new positions
 		this.offset.left = this.startOffset.left + leftDiff;
 		this.offset.top = this.startOffset.top + topDiff;
@@ -326,8 +291,8 @@ $.widget( "ui.draggable", {
 		}
 
 		this.dragEl.css({
-			left: newLeft + "px",
-			top: newTop + "px"
+			left: newLeft,
+			top: newTop
 		});
 	},
 
@@ -337,26 +302,19 @@ $.widget( "ui.draggable", {
 			offset: this.offset
 		};
 
+		// TODO: should we always set the helper?
 		if ( this.options.helper ) {
 			ret.helper = this.dragEl;
 		}
 
 		return ret;
-
 	},
 
 	_unblockFrames: function() {
-	
-		if ( !this.iframeBlocks || !this.iframeBlocks.length ) {
-			return;
+		if ( this.iframeBlocks ) { 
+			this.iframeBlocks.remove();
+			delete this.iframeBlocks;
 		}
-		
-		this.iframeBlocks.each( function() {
-
-			$(this).remove();
-
-		});
-
 	}
 });