Merge branch 'accordion-activate'

5 files changed, 76 insertions, 65 deletions

diff --git a/tests/unit/accordion/accordion_core.js b/tests/unit/accordion/accordion_core.js
index 640427e2e..40a711abf 100644
--- a/tests/unit/accordion/accordion_core.js
+++ b/tests/unit/accordion/accordion_core.js
@@ -22,7 +22,7 @@ test("ui-accordion-heading class added to headers anchor", function() {
 
 test("accessibility", function () {
 	expect(9);
-	var ac = $('#list1').accordion().accordion("activate", 1);
+	var ac = $('#list1').accordion().accordion("option", "active", 1);
 	var headers = $(".ui-accordion-header");
 
 	equals( headers.eq(1).attr("tabindex"), "0", "active header should have tabindex=0");
@@ -32,7 +32,7 @@ test("accessibility", function () {
 	equals( headers.next().attr("role"), "tabpanel", "tabpanel roles");
 	equals( headers.eq(1).attr("aria-expanded"), "true", "active tab has aria-expanded");
 	equals( headers.eq(0).attr("aria-expanded"), "false", "inactive tab has aria-expanded");
-	ac.accordion("activate", 0);
+	ac.accordion("option", "active", 0);
 	equals( headers.eq(0).attr("aria-expanded"), "true", "newly active tab has aria-expanded");
 	equals( headers.eq(1).attr("aria-expanded"), "false", "newly inactive tab has aria-expanded");
 });
diff --git a/tests/unit/accordion/accordion_events.js b/tests/unit/accordion/accordion_events.js
index e9e14996c..4798f7404 100644
--- a/tests/unit/accordion/accordion_events.js
+++ b/tests/unit/accordion/accordion_events.js
@@ -17,14 +17,14 @@ test("accordionchange event, open closed and close again", function() {
 		equals( ui.newHeader.size(), 1 );
 		equals( ui.newContent.size(), 1 );
 	})
-	.accordion("activate", 0)
+	.accordion("option", "active", 0)
 	.one("accordionchange", function(event, ui) {
 		equals( ui.oldHeader.size(), 1 );
 		equals( ui.oldContent.size(), 1 );
 		equals( ui.newHeader.size(), 0 );
 		equals( ui.newContent.size(), 0 );
 	})
-	.accordion("activate", 0);
+	.accordion("option", "active", false);
 });
 
 })(jQuery);
diff --git a/tests/unit/accordion/accordion_methods.js b/tests/unit/accordion/accordion_methods.js
index ed5cb37b6..e46dba30a 100644
--- a/tests/unit/accordion/accordion_methods.js
+++ b/tests/unit/accordion/accordion_methods.js
@@ -50,10 +50,10 @@ test("disable", function() {
 	equals(actual, expected, 'disable is chainable');
 	
 	state(expected, 1, 0, 0)
-	expected.accordion("activate", 1);
+	expected.accordion("option", "active", 1);
 	state(expected, 1, 0, 0)
 	expected.accordion("enable");
-	expected.accordion("activate", 1);
+	expected.accordion("option", "active", 1);
 	state(expected, 0, 1, 0)
 });
 
diff --git a/tests/unit/accordion/accordion_options.js b/tests/unit/accordion/accordion_options.js
index 331ceb924..abfb82d78 100644
--- a/tests/unit/accordion/accordion_options.js
+++ b/tests/unit/accordion/accordion_options.js
@@ -61,7 +61,7 @@ test("{ active: Number }", function() {
 	$('.ui-accordion-header:eq(2)', '#list1').click();
 	equals( $("#list1").accordion('option', 'active'), 2);
 
-	$("#list1").accordion('activate', 0);
+	$("#list1").accordion('option', 'active', 0);
 	equals( $("#list1").accordion('option', 'active'), 0);
 });
 
@@ -96,7 +96,7 @@ test("{ heightStyle: 'content' }", function() {
 });
 test("{ collapsible: false }, default", function() {
 	var ac = $("#list1").accordion();
-	ac.accordion("activate", false);
+	ac.accordion("option", "active", false);
 	state(ac, 1, 0, 0);
 });
 
diff --git a/ui/jquery.ui.accordion.js b/ui/jquery.ui.accordion.js
index c28473529..24ef019e1 100644
--- a/ui/jquery.ui.accordion.js
+++ b/ui/jquery.ui.accordion.js
@@ -73,7 +73,7 @@ $.widget( "ui.accordion", {
 			.addClass( "ui-accordion-content ui-helper-reset ui-widget-content ui-corner-bottom" );
 		self.headers.find( ":first-child" ).addClass( "ui-accordion-heading" );
 
-		self.active = self._findActive( self.active || options.active )
+		self.active = self._findActive( options.active )
 			.addClass( "ui-state-default ui-state-active" )
 			.toggleClass( "ui-corner-all" )
 			.toggleClass( "ui-corner-top" );
@@ -120,7 +120,7 @@ $.widget( "ui.accordion", {
 
 		if ( options.event ) {
 			self.headers.bind( options.event.split(" ").join(".accordion ") + ".accordion", function(event) {
-				self._clickHandler.call( self, event, this );
+				self._eventHandler( event );
 				event.preventDefault();
 			});
 		}
@@ -173,11 +173,14 @@ $.widget( "ui.accordion", {
 	},
 
 	_setOption: function( key, value ) {
-		$.Widget.prototype._setOption.apply( this, arguments );
-		
 		if ( key == "active" ) {
-			this.activate( value );
+			// _activate() will handle invalid values and update this.options
+			this._activate( value );
+			return;
 		}
+		
+		$.Widget.prototype._setOption.apply( this, arguments );
+		
 		if ( key == "icons" ) {
 			this._destroyIcons();
 			if ( value ) {
@@ -213,7 +216,7 @@ $.widget( "ui.accordion", {
 				break;
 			case keyCode.SPACE:
 			case keyCode.ENTER:
-				this._clickHandler( { target: event.target }, event.target );
+				this._eventHandler( event );
 				event.preventDefault();
 		}
 
@@ -272,73 +275,64 @@ $.widget( "ui.accordion", {
 		return this;
 	},
 
-	activate: function( index ) {
-		// TODO this gets called on init, changing the option without an explicit call for that
-		this.options.active = index;
-		// call clickHandler with custom event
+	_activate: function( index ) {
 		var active = this._findActive( index )[ 0 ];
-		this._clickHandler( { target: active }, active );
 
-		return this;
+		// we found a header to activate, just delegate to the event handler
+		if ( active ) {
+			if ( active !== this.active[ 0 ] ) {
+				this._eventHandler( { target: active, currentTarget: active } );
+			}
+			return;
+		}
+
+		// no header to activate, check if we can collapse
+		if ( !this.options.collapsible ) {
+			return;
+		}
+
+		this.active
+			.removeClass( "ui-state-active ui-corner-top" )
+			.addClass( "ui-state-default ui-corner-all" )
+			.children( ".ui-icon" )
+				.removeClass( this.options.icons.activeHeader )
+				.addClass( this.options.icons.header );
+		this.active.next().addClass( "ui-accordion-content-active" );
+		var toHide = this.active.next(),
+			data = {
+				options: this.options,
+				newHeader: $( [] ),
+				oldHeader: this.active,
+				newContent: $( [] ),
+				oldContent: toHide
+			},
+			toShow = ( this.active = $( [] ) );
+		this._toggle( toShow, toHide, data );
 	},
 
+	// TODO: add tests/docs for negative values in 2.0 (#6854)
 	_findActive: function( selector ) {
-		return selector
-			? typeof selector === "number"
-				? this.headers.filter( ":eq(" + selector + ")" )
-				: this.headers.not( this.headers.not( selector ) )
-			: selector === false
-				? $( [] )
-				: this.headers.filter( ":eq(0)" );
+		return typeof selector === "number" ? this.headers.eq( selector ) : $( [] );
 	},
 
-	// TODO isn't event.target enough? why the separate target argument?
-	_clickHandler: function( event, target ) {
-		var options = this.options;
+	_eventHandler: function( event ) {
+		var options = this.options,
+			clicked = $( event.currentTarget ),
+			clickedIsActive = clicked[0] === this.active[0];
+
 		if ( options.disabled ) {
 			return;
 		}
 
-		// called only when using activate(false) to close all parts programmatically
-		if ( !event.target ) {
-			if ( !options.collapsible ) {
-				return;
-			}
-			this.active
-				.removeClass( "ui-state-active ui-corner-top" )
-				.addClass( "ui-state-default ui-corner-all" )
-				.children( ".ui-icon" )
-					.removeClass( options.icons.activeHeader )
-					.addClass( options.icons.header );
-			this.active.next().addClass( "ui-accordion-content-active" );
-			var toHide = this.active.next(),
-				data = {
-					options: options,
-					newHeader: $( [] ),
-					oldHeader: options.active,
-					newContent: $( [] ),
-					oldContent: toHide
-				},
-				toShow = ( this.active = $( [] ) );
-			this._toggle( toShow, toHide, data );
+		// if animations are still active, or the active header is the target, ignore click
+		if ( this.running || ( !options.collapsible && clickedIsActive ) ) {
 			return;
 		}
 
-		// get the click target
-		var clicked = $( event.currentTarget || target ),
-			clickedIsActive = clicked[0] === this.active[0];
-
-		// TODO the option is changed, is that correct?
-		// TODO if it is correct, shouldn't that happen after determining that the click is valid?
 		options.active = options.collapsible && clickedIsActive ?
 			false :
 			this.headers.index( clicked );
 
-		// if animations are still active, or the active header is the target, ignore click
-		if ( this.running || ( !options.collapsible && clickedIsActive ) ) {
-			return;
-		}
-
 		// find elements to show and hide
 		var active = this.active,
 			toShow = clicked.next(),
@@ -375,8 +369,6 @@ $.widget( "ui.accordion", {
 				.next()
 				.addClass( "ui-accordion-content-active" );
 		}
-
-		return;
 	},
 
 	_toggle: function( toShow, toHide, data, clickedIsActive, down ) {
@@ -685,4 +677,23 @@ $.extend( $.ui.accordion, {
 	};
 }( jQuery, jQuery.ui.accordion.prototype ) );
 
+// expanded active option, activate method
+(function( $, prototype ) {
+	prototype.activate = prototype._activate;
+
+	var _findActive = prototype._findActive;
+	prototype._findActive = function( index ) {
+		if ( index === -1 ) {
+			index = false;
+		}
+		if ( index && typeof index !== "number" ) {
+			index = this.headers.index( this.headers.filter( index ) );
+			if ( index === -1 ) {
+				index = false;
+			}
+		}
+		return _findActive.call( this, index );
+	};
+}( jQuery, jQuery.ui.accordion.prototype ) );
+
 })( jQuery );