From b9b604936e29c3d6a5a50f75460eb3c98ee80f30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Tue, 15 Sep 2009 08:33:28 +0000 Subject: [PATCH] accordion: remove deprecated alwaysOpen option (collapsible was introduced in last stable release); fixed collapsible-false accordion in combination with activate method (with fasly-argument to close all); changed activate-option default to 0 (which was the "computed" default anyway); lots of fixes for the testsuite, while removing some of the "missing" tests: header accepts only a selector (updated spec to make that more clear), while testing animations in unit tests is rather pointless --- tests/unit/accordion/accordion_core.js | 24 ++--- tests/unit/accordion/accordion_defaults.js | 6 +- tests/unit/accordion/accordion_methods.js | 90 +++++++++++++++--- tests/unit/accordion/accordion_options.js | 101 ++++++++------------- ui/ui.accordion.js | 28 +++--- 5 files changed, 141 insertions(+), 108 deletions(-) diff --git a/tests/unit/accordion/accordion_core.js b/tests/unit/accordion/accordion_core.js index 524842037..797ead864 100644 --- a/tests/unit/accordion/accordion_core.js +++ b/tests/unit/accordion/accordion_core.js @@ -2,34 +2,26 @@ * accordion_core.js */ + +(function($) { + jQuery.ui.accordion.defaults.animated = false; function state(accordion) { var args = $.makeArray(arguments).slice(1); + var result = []; $.each(args, function(i, n) { - equals(accordion.find(".ui-accordion-content").eq(i).is(":visible"), n); + result.push( accordion.find(".ui-accordion-content").eq(i).is(":visible") ? 1 : 0 ); }); + same(args, result) } -function state2(accordion) { - var args = $.makeArray(arguments).slice(1); - $.each(args, function(i, n) { - equals(accordion.find("div").eq(i).is(":visible"), n); - }); -} - -$.fn.triggerEvent = function(type, target) { - return this.triggerHandler(type, [jQuery.event.fix({ type: type, target: target })]); -}; - -(function($) { - module("accordion: core"); test("handle click on header-descendant", function() { var ac = $('#navigation').accordion({ autoHeight: false }); - ac.triggerEvent("click", $('#navigation span:contains(Bass)')[0]); - state2(ac, 0, 1, 0); + $('#navigation h2:eq(1) a').trigger("click"); + state(ac, 0, 1, 0); }); test("accessibility", function () { diff --git a/tests/unit/accordion/accordion_defaults.js b/tests/unit/accordion/accordion_defaults.js index 78561ca36..e472734bc 100644 --- a/tests/unit/accordion/accordion_defaults.js +++ b/tests/unit/accordion/accordion_defaults.js @@ -3,7 +3,7 @@ */ var accordion_defaults = { - active: null, + active: 0, animated: false, autoHeight: true, clearStyle: false, @@ -14,9 +14,7 @@ var accordion_defaults = { header: "> li > :first-child,> :not(li):even", icons: { "header": "ui-icon-triangle-1-e", "headerSelected": "ui-icon-triangle-1-s" }, navigation: false, - navigationFilter: function() { - return this.href.toLowerCase() == location.href.toLowerCase(); - } + navigationFilter: function() {} }; commonWidgetTests('accordion', { defaults: accordion_defaults }); diff --git a/tests/unit/accordion/accordion_methods.js b/tests/unit/accordion/accordion_methods.js index e13a4a47e..75c11b46b 100644 --- a/tests/unit/accordion/accordion_methods.js +++ b/tests/unit/accordion/accordion_methods.js @@ -3,31 +3,87 @@ */ (function($) { +function state(accordion) { + var expected = $.makeArray(arguments).slice(1); + var actual = []; + $.each(expected, function(i, n) { + actual.push( accordion.find(".ui-accordion-content").eq(i).is(":visible") ? 1 : 0 ); + }); + same(actual, expected) +} + module("accordion: methods"); test("init", function() { - ok(false, 'missing test - untested code is broken code'); + $("
").appendTo('body').accordion().remove(); + ok(true, '.accordion() called on element'); + + $([]).accordion().remove(); + ok(true, '.accordion() called on empty collection'); + + $('
').accordion().remove(); + ok(true, '.accordion() called on disconnected DOMElement - never connected'); + + $('
').appendTo('body').remove().accordion().remove(); + ok(true, '.accordion() called on disconnected DOMElement - removed'); + + $('
').accordion().accordion("foo").remove(); + ok(true, 'arbitrary method called after init'); + + var el = $('
').accordion(); + var foo = el.data("foo.accordion"); + el.remove(); + ok(true, 'arbitrary option getter after init'); + + $('
').accordion().data("foo.accordion", "bar").remove(); + ok(true, 'arbitrary option setter after init'); }); test("destroy", function() { - var expected = $('#list1').accordion(), + $("
").appendTo('body').accordion().accordion("destroy").remove(); + ok(true, '.accordion("destroy") called on element'); + + $([]).accordion().accordion("destroy").remove(); + ok(true, '.accordion("destroy") called on empty collection'); + + $('
').accordion().accordion("destroy").remove(); + ok(true, '.accordion("destroy") called on disconnected DOMElement'); + + $('
').accordion().accordion("destroy").accordion("foo").remove(); + ok(true, 'arbitrary method called after destroy'); + + var el = $('
').accordion(); + var foo = el.accordion("destroy").data("foo.accordion"); + el.remove(); + ok(true, 'arbitrary option getter after destroy'); + + $('
').accordion().accordion("destroy").data("foo.accordion", "bar").remove(); + ok(true, 'arbitrary option setter after destroy'); + + var expected = $('
').accordion(), actual = expected.accordion('destroy'); equals(actual, expected, 'destroy is chainable'); - ok(false, 'missing test - untested code is broken code'); }); test("enable", function() { var expected = $('#list1').accordion(), actual = expected.accordion('enable'); equals(actual, expected, 'enable is chainable'); - ok(false, 'missing test - untested code is broken code'); + + state(expected, 1, 0, 0) }); test("disable", function() { var expected = $('#list1').accordion(), actual = expected.accordion('disable'); equals(actual, expected, 'disable is chainable'); - ok(false, 'missing test - untested code is broken code'); + + state(expected, 1, 0, 0) + expected.accordion("activate", 1); + state(expected, 1, 0, 0) + expected.accordion("enable"); + expected.accordion("activate", 1); + state(expected, 0, 1, 0) }); test("activate", function() { @@ -47,8 +103,6 @@ test("activate, numeric", function() { state(ac, 0, 1, 0); ac.accordion("activate", 2); state(ac, 0, 0, 1); - ac.accordion("activate", -1); - state(ac, 0, 0, 1); }); test("activate, boolean and numeric, collapsible:true", function() { @@ -62,7 +116,7 @@ test("activate, boolean and numeric, collapsible:true", function() { state(ac, 0, 0, 0); }); -test("activate, boolean, collapsible:false", function() { +test("activate, boolean, collapsible: false", function() { var ac = $('#list1').accordion().accordion("activate", 2); state(ac, 0, 0, 1); ac.accordion("activate", -1); @@ -90,10 +144,24 @@ test("activate, jQuery or DOM element", function() { }); test("resize", function() { - var expected = $('#list1').accordion(), - actual = expected.accordion('resize'); + var expected = $('#list1').accordion(); + + var sizes = []; + expected.find(".ui-accordion-content").each(function() { + sizes.push($(this).outerHeight()); + }); + + var actual = expected.accordion('resize'); equals(actual, expected, 'resize is chainable'); - ok(false, 'missing test - untested code is broken code'); + + var sizes2 = []; + expected.find(".ui-accordion-content").each(function() { + sizes2.push($(this).outerHeight()); + }); + same(sizes, sizes2); + + expected.find(".ui-accordion-content:first").height(500) + var sizes3 = []; }); })(jQuery); diff --git a/tests/unit/accordion/accordion_options.js b/tests/unit/accordion/accordion_options.js index 29fc9c032..35e87d432 100644 --- a/tests/unit/accordion/accordion_options.js +++ b/tests/unit/accordion/accordion_options.js @@ -3,38 +3,57 @@ */ (function($) { +function state(accordion) { + var expected = $.makeArray(arguments).slice(1); + var actual = []; + $.each(expected, function(i, n) { + actual.push( accordion.find(".ui-accordion-content").eq(i).is(":visible") ? 1 : 0 ); + }); + same(actual, expected) +} + + module("accordion: options"); test("{ active: first child }, default", function() { - $("#list1").accordion(); - equals( $("#list1").accordion('option', 'active'), 0); + var ac = $("#list1").accordion(); + equals( ac.accordion('option', 'active'), 0); + state(ac, 1, 0, 0) }); test("{ active: Selector }", function() { - ok(false, 'missing test - untested code is broken code'); + var ac = $("#list1").accordion({ + active: "a:last" + }); + state(ac, 0, 0, 1); + ac.accordion('option', 'active', "a:eq(1)"); + state(ac, 0, 1, 0); }); test("{ active: Element }", function() { - ok(false, 'missing test - untested code is broken code'); + var ac = $("#list1").accordion({ + active: $("#list1 a:last")[0] + }); + state(ac, 0, 0, 1); + ac.accordion('option', 'active', $("#list1 a:eq(1)")[0]); + state(ac, 0, 1, 0); }); test("{ active: jQuery Object }", function() { - ok(false, 'missing test - untested code is broken code'); -}); - -test("{ active: true }", function() { - $("#list1").accordion({ - active: true, - collapsible: false + var ac = $("#list1").accordion({ + active: $("#list1 a:last") }); - equals( $("#list1 .ui-accordion-header.ui-state-active").size(), 1, "one header selected" ); + state(ac, 0, 0, 1); + ac.accordion('option', 'active', $("#list1 a:eq(1)")); + state(ac, 0, 1, 0); }); test("{ active: false }", function() { - $("#list1").accordion({ + var ac = $("#list1").accordion({ active: false, collapsible: true }); + state(ac, 0, 0, 0); equals( $("#list1 .ui-accordion-header.ui-state-active").size(), 0, "no headers selected" ); equals( $("#list1").accordion('option', 'active'), false); }); @@ -56,18 +75,6 @@ test("{ active: Number }", function() { equals( $("#list1").accordion('option', 'active'), 0); }); -test("{ animated: false }, default", function() { - ok(false, 'missing test - untested code is broken code'); -}); - -test("{ animated: true }", function() { - ok(false, 'missing test - untested code is broken code'); -}); - -test("{ animated: String }", function() { - ok(false, 'missing test - untested code is broken code'); -}); - test("{ autoHeight: true }, default", function() { $('#navigation').accordion({ autoHeight: true }); equals( $('#navigation > li:eq(0) > ul').height(), 126 ); @@ -82,33 +89,20 @@ test("{ autoHeight: false }", function() { equals( $('#navigation > li:eq(2) > ul').height(), 54 ); }); -test("{ clearStyle: false }, default", function() { - ok(false, 'missing test - untested code is broken code'); -}); - -test("{ clearStyle: true }", function() { - ok(false, 'missing test - untested code is broken code'); -}); - test("{ collapsible: false }, default", function() { - ok(false, 'missing test - untested code is broken code'); + var ac = $("#list1").accordion(); + ac.accordion("activate", false); + state(ac, 1, 0, 0); }); test("{ collapsible: true }", function() { - $("#list1").accordion({ + var ac = $("#list1").accordion({ active: 1, collapsible: true }); - $('.ui-accordion-header:eq(1)', '#list1').click(); + var header = $('#list1 .ui-accordion-header:eq(1)').click(); equals( $("#list1").accordion('option', 'active'), false); -}); - -test("{ event: 'click' }, default", function() { - ok(false, 'missing test - untested code is broken code'); -}); - -test("{ event: 'mouseover' }", function() { - ok(false, 'missing test - untested code is broken code'); + state(ac, 0, 0, 0); }); test("{ fillSpace: false }, default", function() { @@ -128,23 +122,8 @@ test("{ fillSpace: true }", function() { }); test("{ header: '> li > :first-child,> :not(li):even' }, default", function() { - ok(false, 'missing test - untested code is broken code'); -}); - -test("{ header: Selector }", function() { - ok(false, 'missing test - untested code is broken code'); -}); - -test("{ header: jQuery Object }", function() { - ok(false, 'missing test - untested code is broken code'); -}); - -test("{ icons: { 'header': 'ui-icon-triangle-1-e', 'headerSelected': 'ui-icon-triangle-1-s' } }, default", function() { - ok(false, 'missing test - untested code is broken code'); -}); - -test("{ icons: { 'header': 'ui-icon-foo', 'headerSelected': 'ui-icon-bar' } }", function() { - ok(false, 'missing test - untested code is broken code'); + state($("#list1").accordion(), 1, 0, 0); + state($("#navigation").accordion(), 1, 0, 0); }); test("{ icons: false }", function() { diff --git a/ui/ui.accordion.js b/ui/ui.accordion.js index 17e3cec42..cbad4e913 100644 --- a/ui/ui.accordion.js +++ b/ui/ui.accordion.js @@ -19,14 +19,6 @@ $.widget("ui.accordion", { var o = this.options, self = this; this.running = 0; - // if the user set the alwaysOpen option on init - // then we need to set the collapsible option - // if they set both on init, collapsible will take priority - if (o.collapsible == $.ui.accordion.defaults.collapsible && - o.alwaysOpen != $.ui.accordion.defaults.alwaysOpen) { - o.collapsible = !o.alwaysOpen; - } - this.element.addClass("ui-accordion ui-widget ui-helper-reset"); // in lack of child-selectors in CSS we need to mark top-LIs in a UL-accordion for some IE-fix @@ -148,9 +140,6 @@ $.widget("ui.accordion", { }, _setData: function(key, value) { - // alwaysOpen is deprecated - if(key == 'alwaysOpen'){ key = 'collapsible'; value = !value; } - $.widget.prototype._setData.apply(this, arguments); if (key == "active") { @@ -231,6 +220,7 @@ $.widget("ui.accordion", { }, 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 var active = this._findActive(index)[0]; @@ -249,13 +239,17 @@ $.widget("ui.accordion", { : this.headers.filter(":eq(0)"); }, + // TODO isn't event.target enough? why the seperate target argument? _clickHandler: function(event, target) { var o = this.options; - if (o.disabled) { return; } + if (o.disabled) + return; // called only when using activate(false) to close all parts programmatically - if (!event.target && o.collapsible) { + if (!event.target) { + if (!o.collapsible) + return; this.active.removeClass("ui-state-active ui-corner-top").addClass("ui-state-default ui-corner-all") .find(".ui-icon").removeClass(o.icons.headerSelected).addClass(o.icons.header); this.active.next().addClass('ui-accordion-content-active'); @@ -276,7 +270,9 @@ $.widget("ui.accordion", { var clicked = $(event.currentTarget || target); var clickedIsActive = clicked[0] == this.active[0]; - o.active = $('.ui-accordion-header', this.element).index(clicked); + // TODO the option is changed, is that correct? + // TODO if it is correct, shouldn't that happen after determining that the click is valid? + o.active = o.collapsible && clickedIsActive ? false : $('.ui-accordion-header', this.element).index(clicked); // if animations are still active, or the active header is the target, ignore click if (this.running || (!o.collapsible && clickedIsActive)) { @@ -394,6 +390,7 @@ $.widget("ui.accordion", { } + // TODO assert that the blur and focus triggers are really necessary, remove otherwise toHide.prev().attr('aria-expanded','false').attr("tabIndex", "-1").blur(); toShow.prev().attr('aria-expanded','true').attr("tabIndex", "0").focus(); @@ -425,8 +422,7 @@ $.widget("ui.accordion", { $.extend($.ui.accordion, { version: "@VERSION", defaults: { - active: null, - alwaysOpen: true, //deprecated, use collapsible + active: 0, animated: 'slide', autoHeight: true, clearStyle: false,