From 8b23483c0b03369dbfc9720e2aff8de690a3b854 Mon Sep 17 00:00:00 2001 From: Scott González Date: Tue, 11 Jan 2011 20:53:31 -0500 Subject: Accordion: First pass at deprecating the activate method. Renamed _clickHandler to _eventHandler and removed extraneous parameter. Updated all tests to use the option methods instead of the activate method. --- tests/unit/accordion/accordion_core.js | 4 ++-- tests/unit/accordion/accordion_events.js | 4 ++-- tests/unit/accordion/accordion_methods.js | 4 ++-- tests/unit/accordion/accordion_options.js | 4 ++-- ui/jquery.ui.accordion.js | 23 +++++++++++------------ 5 files changed, 19 insertions(+), 20 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..5793e46d0 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", 0); }); })(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 f7d0e27b2..e58c5d621 100644 --- a/ui/jquery.ui.accordion.js +++ b/ui/jquery.ui.accordion.js @@ -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(); }); } @@ -176,7 +176,7 @@ $.widget( "ui.accordion", { $.Widget.prototype._setOption.apply( this, arguments ); if ( key == "active" ) { - this.activate( value ); + this._activate( value ); } if ( key == "icons" ) { this._destroyIcons(); @@ -213,7 +213,7 @@ $.widget( "ui.accordion", { break; case keyCode.SPACE: case keyCode.ENTER: - this._clickHandler( { target: event.target }, event.target ); + this._eventHandler( event ); event.preventDefault(); } @@ -272,14 +272,11 @@ $.widget( "ui.accordion", { return this; }, - activate: function( index ) { - // TODO this gets called on init, changing the option without an explicit call for that + _activate: function( index ) { + // TODO: handle invalid values this.options.active = index; - // call clickHandler with custom event var active = this._findActive( index )[ 0 ]; - this._clickHandler( { target: active }, active ); - - return this; + this._eventHandler( { target: active, currentTarget: active } ); }, _findActive: function( selector ) { @@ -292,8 +289,7 @@ $.widget( "ui.accordion", { : this.headers.filter( ":eq(0)" ); }, - // TODO isn't event.target enough? why the separate target argument? - _clickHandler: function( event, target ) { + _eventHandler: function( event ) { var options = this.options; if ( options.disabled ) { return; @@ -325,7 +321,7 @@ $.widget( "ui.accordion", { } // get the click target - var clicked = $( event.currentTarget || target ), + var clicked = $( event.currentTarget ), clickedIsActive = clicked[0] === this.active[0]; // TODO the option is changed, is that correct? @@ -683,4 +679,7 @@ $.extend( $.ui.accordion, { }; }( jQuery, jQuery.ui.accordion.prototype ) ); +// activate method +jQuery.ui.accordion.prototype.activate = jQuery.ui.accordion.prototype._activate; + })( jQuery ); -- cgit v1.2.3 From 368af59137f3dcfd7916a043868c0f1827f761fb Mon Sep 17 00:00:00 2001 From: Scott González Date: Tue, 11 Jan 2011 21:24:41 -0500 Subject: Accordion: Handle invalid values for the active option. --- ui/jquery.ui.accordion.js | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/ui/jquery.ui.accordion.js b/ui/jquery.ui.accordion.js index e58c5d621..e0acd76a8 100644 --- a/ui/jquery.ui.accordion.js +++ b/ui/jquery.ui.accordion.js @@ -173,11 +173,14 @@ $.widget( "ui.accordion", { }, _setOption: function( key, value ) { - $.Widget.prototype._setOption.apply( this, arguments ); - if ( key == "active" ) { + // _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 ) { @@ -273,20 +276,28 @@ $.widget( "ui.accordion", { }, _activate: function( index ) { - // TODO: handle invalid values - this.options.active = index; var active = this._findActive( index )[ 0 ]; + if ( !active ) { + if ( !this.options.collapsible ) { + return; + } + index = false; + } + this.options.active = index; this._eventHandler( { target: active, currentTarget: active } ); }, _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)" ); + // handle -1 separately, we should drop support for this + // so that we can allow selecting via negative index, like .eq() + if ( selector === -1 ) { + selector = undefined; + } + return typeof selector === "number" ? + this.headers.eq( selector ) : + selector ? + this.headers.filter( selector ) : + $( [] ); }, _eventHandler: function( event ) { -- cgit v1.2.3 From b5b8cefcb0f9e253a96c8aa97125b6353ebf2dcd Mon Sep 17 00:00:00 2001 From: Scott González Date: Thu, 13 Jan 2011 08:51:09 -0500 Subject: Accordion: Reduced valid active options to numbers and falsey. Added compatibility layer for previously allowed values. --- ui/jquery.ui.accordion.js | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/ui/jquery.ui.accordion.js b/ui/jquery.ui.accordion.js index e0acd76a8..bcc798bf8 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" ); @@ -287,17 +287,9 @@ $.widget( "ui.accordion", { this._eventHandler( { target: active, currentTarget: active } ); }, + // TODO: add tests for negative values in 2.0 _findActive: function( selector ) { - // handle -1 separately, we should drop support for this - // so that we can allow selecting via negative index, like .eq() - if ( selector === -1 ) { - selector = undefined; - } - return typeof selector === "number" ? - this.headers.eq( selector ) : - selector ? - this.headers.filter( selector ) : - $( [] ); + return typeof selector === "number" ? this.headers.eq( selector ) : $( [] ); }, _eventHandler: function( event ) { @@ -690,7 +682,23 @@ $.extend( $.ui.accordion, { }; }( jQuery, jQuery.ui.accordion.prototype ) ); -// activate method -jQuery.ui.accordion.prototype.activate = jQuery.ui.accordion.prototype._activate; +// 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 ); -- cgit v1.2.3 From 3c11cd3051b463c4e18b5b5af1d0cac7f65c0537 Mon Sep 17 00:00:00 2001 From: Scott González Date: Thu, 13 Jan 2011 14:01:38 -0500 Subject: Accordion: Added note about supporting negative values for active option in 2.0. --- ui/jquery.ui.accordion.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/jquery.ui.accordion.js b/ui/jquery.ui.accordion.js index bcc798bf8..eb6cdb5d0 100644 --- a/ui/jquery.ui.accordion.js +++ b/ui/jquery.ui.accordion.js @@ -287,7 +287,7 @@ $.widget( "ui.accordion", { this._eventHandler( { target: active, currentTarget: active } ); }, - // TODO: add tests for negative values in 2.0 + // TODO: add tests/docs for negative values in 2.0 (#6854) _findActive: function( selector ) { return typeof selector === "number" ? this.headers.eq( selector ) : $( [] ); }, -- cgit v1.2.3 From 468c35877aa7db472ae191617e32069a0e656ec7 Mon Sep 17 00:00:00 2001 From: Scott González Date: Thu, 13 Jan 2011 14:42:35 -0500 Subject: Accordion: Moved handling for programmatically collapsing the accordion out of the event handler. Modified event handler to not change the active option until after it determines that the event is valid. --- tests/unit/accordion/accordion_events.js | 2 +- ui/jquery.ui.accordion.js | 77 +++++++++++++++----------------- 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/tests/unit/accordion/accordion_events.js b/tests/unit/accordion/accordion_events.js index 5793e46d0..4798f7404 100644 --- a/tests/unit/accordion/accordion_events.js +++ b/tests/unit/accordion/accordion_events.js @@ -24,7 +24,7 @@ test("accordionchange event, open closed and close again", function() { equals( ui.newHeader.size(), 0 ); equals( ui.newContent.size(), 0 ); }) - .accordion("option", "active", 0); + .accordion("option", "active", false); }); })(jQuery); diff --git a/ui/jquery.ui.accordion.js b/ui/jquery.ui.accordion.js index eb6cdb5d0..61587ac00 100644 --- a/ui/jquery.ui.accordion.js +++ b/ui/jquery.ui.accordion.js @@ -277,14 +277,37 @@ $.widget( "ui.accordion", { _activate: function( index ) { var active = this._findActive( index )[ 0 ]; - if ( !active ) { - if ( !this.options.collapsible ) { - return; + + // 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 } ); } - index = false; + return; } - this.options.active = index; - this._eventHandler( { target: active, currentTarget: active } ); + + // 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) @@ -293,51 +316,23 @@ $.widget( "ui.accordion", { }, _eventHandler: function( event ) { - var options = this.options; + 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 ), - 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(), @@ -374,8 +369,6 @@ $.widget( "ui.accordion", { .next() .addClass( "ui-accordion-content-active" ); } - - return; }, _toggle: function( toShow, toHide, data, clickedIsActive, down ) { -- cgit v1.2.3