r78915 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r78914‎ | r78915 | r78916 >
Date:19:26, 23 December 2010
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
whitespace according to mw conventions + using mw.msg instead of english hardcoding
Modified paths:
  • /trunk/phase3/resources/jquery/jquery.makeCollapsible.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/jquery/jquery.makeCollapsible.js
@@ -1,5 +1,5 @@
22 /**
3 - * make lists, divs, tables etc. toggleable
 3+ * jQuery makeCollapsible
44 *
55 * This will enable collapsible-functionality on all passed elements.
66 * Will prevent binding twice to the same element.
@@ -8,102 +8,113 @@
99 * Elements made collapsible have class "kr-made-collapsible".
1010 * Except for tables and lists, the inner content is wrapped in "kr-collapsible-content".
1111 *
12 - * Copyright 2008-2009 Krinkle <krinklemail@gmail.com>
 12+ * @author Krinkle <krinklemail@gmail.com>
1313 *
14 - * @license CC-BY 2.0 NL <http://creativecommons.org/licenses/by/2.0/nl/deed.en>
 14+ * @TODO: Remove old "kr-" prefix
 15+ *
 16+ * Dual license:
 17+ * @license CC-BY 3.0 <http://creativecommons.org/licenses/by/3.0>
 18+ * @license GPL2 <http://www.gnu.org/licenses/old-licenses/gpl-2.0.html>
1519 */
1620
1721 $.fn.makeCollapsible = function() {
1822
1923 return this.each(function() {
20 - var $that = $(this).addClass('kr-collapsible'), // in case $('#myAJAXelement').makeCollapsible() was called
 24+ var $that = $(this).addClass( 'kr-collapsible' ), // in case $( '#myAJAXelement' ).makeCollapsible() was called
2125 that = this,
22 - collapsetext = $(this).attr('data-collapsetext'),
23 - expandtext = $(this).attr('data-expandtext');
 26+ collapsetext = $(this).attr( 'data-collapsetext' ),
 27+ expandtext = $(this).attr( 'data-expandtext' );
2428 // Use custom text or default ?
25 - if( !collapsetext || collapsetext == ''){
26 - collapsetext = 'Collapse';
 29+ if( !collapsetext || collapsetext == '' ){
 30+ collapsetext = mw.msg( 'hide' );
2731 }
28 - if ( !expandtext || expandtext == ''){
29 - expandtext = 'Expand';
 32+ if ( !expandtext || expandtext == '' ){
 33+ expandtext = mw.msg( 'show' );
3034 }
3135 // Create toggle link with a space around the brackets (&nbsp)
32 - $toggleLink = $('<span class="kr-collapsible-toggle kr-collapsible-toggle-hide">&nbsp;[<a href="#">' + collapsetext + '</a>]&nbsp;</span>').click(function(){
 36+ $toggleLink = $( '<span class="kr-collapsible-toggle kr-collapsible-toggle-hide">&nbsp;[<a href="#">' + collapsetext + '</a>]&nbsp;</span>' ).click( function(){
3337 var $that = $(this),
34 - $collapsible = $that.closest('.kr-collapsible.kr-made-collapsible').toggleClass('kr-collapsed');
35 - if ($that.hasClass('kr-collapsible-toggle-hide')) {
 38+ $collapsible = $that.closest( '.kr-collapsible.kr-made-collapsible' ).toggleClass( 'kr-collapsed' );
 39+ if ( $that.hasClass( 'kr-collapsible-toggle-hide' ) ) {
3640 // Change link to "Show"
3741 $that
38 - .removeClass('kr-collapsible-toggle-hide').addClass('kr-collapsible-toggle-show')
39 - .find('> a').html(expandtext);
 42+ .removeClass( 'kr-collapsible-toggle-hide' ).addClass( 'kr-collapsible-toggle-show' )
 43+ .find( '> a' ).html( expandtext );
4044 // Hide the collapsible element
41 - if ($collapsible.is('table')) {
 45+ if ( $collapsible.is( 'table' ) ) {
4246 // Hide all direct childing table rows of this table, except the row containing the link
4347 // Slide doens't work, but fade works fine as of jQuery 1.1.3
4448 // http://stackoverflow.com/questions/467336/jquery-how-to-use-slidedown-or-show-function-on-a-table-row#920480
4549 // Stop to prevent animaties from stacking up
46 - $collapsible.find('> tbody > tr').not($that.parent().parent()).stop(true, true).fadeOut();
47 - } else if ($collapsible.is('ul') || $collapsible.is('ol')) {
48 - $collapsible.find('> li').not($that.parent()).stop(true, true).slideUp();
 50+ $collapsible.find( '> tbody > tr' ).not( $that.parent().parent() ).stop(true, true).fadeOut();
 51+
 52+ } else if ( $collapsible.is( 'ul' ) || $collapsible.is( 'ol' ) ) {
 53+ $collapsible.find( '> li' ).not($that.parent()).stop(true, true).slideUp();
 54+
4955 } else { // <div>, <p> etc.
50 - $collapsible.find('> .kr-collapsible-content').slideUp();
 56+ $collapsible.find( '> .kr-collapsible-content' ).slideUp();
5157 }
 58+
5259 } else {
5360 // Change link to "Hide"
5461 $that
55 - .removeClass('kr-collapsible-toggle-show').addClass('kr-collapsible-toggle-hide')
56 - .find('> a').html(collapsetext);
 62+ .removeClass( 'kr-collapsible-toggle-show' ).addClass( 'kr-collapsible-toggle-hide' )
 63+ .find( '> a' ).html( collapsetext );
5764 // Show the collapsible element
58 - if ($collapsible.is('table')) {
59 - $collapsible.find('> tbody > tr').not($that.parent().parent()).stop(true, true).fadeIn();
60 - } else if ($collapsible.is('ul') || $collapsible.is('ol')) {
61 - $collapsible.find('> li').not($that.parent()).stop(true, true).slideDown();
 65+ if ( $collapsible.is( 'table' ) ) {
 66+ $collapsible.find( '> tbody > tr' ).not( $that.parent().parent() ).stop(true, true).fadeIn();
 67+
 68+ } else if ( $collapsible.is( 'ul' ) || $collapsible.is( 'ol' ) ) {
 69+ $collapsible.find( '> li' ).not( $that.parent() ).stop(true, true).slideDown();
 70+
6271 } else { // <div>, <p> etc.
63 - $collapsible.find('> .kr-collapsible-content').slideDown();
 72+ $collapsible.find( '> .kr-collapsible-content' ).slideDown();
6473 }
6574 }
6675 return false;
67 - });
 76+ } );
 77+
6878 // Skip if it has been enabled already.
69 - if ($that.hasClass('kr-made-collapsible')) {
 79+ if ($that.hasClass( 'kr-made-collapsible' )) {
7080 return;
7181 } else {
72 - $that.addClass('kr-made-collapsible');
 82+ $that.addClass( 'kr-made-collapsible' );
7383 }
 84+
7485 // Elements are treated differently
75 - if ($that.is('table')) {
 86+ if ( $that.is( 'table' ) ) {
7687 // The toggle-link will be in the last cell (td or th) of the first row
77 - var $lastCell = $('tr:first th, tr:first td', that).eq(-1);
78 - if (!$lastCell.find('> .kr-collapsible-toggle').size()) {
79 - $lastCell.prepend($toggleLink);
 88+ var $lastCell = $( 'tr:first th, tr:first td', that ).eq(-1);
 89+ if ( !$lastCell.find( '> .kr-collapsible-toggle' ).size() ) {
 90+ $lastCell.prepend( $toggleLink );
8091 }
8192
82 - } else if ($that.is('ul') || $that.is('ol')) {
83 - if (!$('li:first', $that).find('> .kr-collapsible-toggle').size()) {
 93+ } else if ($that.is( 'ul' ) || $that.is( 'ol' )) {
 94+ if ( !$( 'li:first', $that).find( '> .kr-collapsible-toggle' ).size() ) {
8495 // Make sure the numeral count doesn't get messed up, reset to 1 unless value-attribute is already used
85 - if ( $('li:first', $that).attr('value') == '' ) {
86 - $('li:first', $that).attr('value', '1');
 96+ if ( $( 'li:first', $that ).attr( 'value' ) == '' ) {
 97+ $( 'li:first', $that ).attr( 'value', '1' );
8798 }
88 - $that.prepend($toggleLink.wrap('<li class="kr-collapsibile-toggle-li">').parent());
 99+ $that.prepend( $toggleLink.wrap( '<li class="kr-collapsibile-toggle-li">' ).parent() );
89100 }
90101 } else { // <div>, <p> etc.
91102 // Is there an content-wrapper already made ?
92103 // If a direct child with the class does not exists, create the wrap.
93 - if (!$that.find('g> .kr-collapsible-content').size()) {
94 - $that.wrapInner('<div class="kr-collapsible-content">');
 104+ if ( !$that.find( '> .kr-collapsible-content' ).size() ) {
 105+ $that.wrapInner( '<div class="kr-collapsible-content">' );
95106 }
96107
97108 // Add toggle-link if not present
98 - if (!$that.find('> .kr-collapsible-toggle').size()) {
99 - $that.prepend($toggleLink);
 109+ if ( !$that.find( '> .kr-collapsible-toggle' ).size() ) {
 110+ $that.prepend( $toggleLink );
100111 }
101112 }
102113 // Initial state
103 - if ($that.hasClass('kr-collapsed')) {
 114+ if ( $that.hasClass( 'kr-collapsed' ) ) {
104115 $toggleLink.click();
105116 }
106 - });
 117+ } );
107118 };
108 -$(function(){
109 - $('.kr-collapsible').makeCollapsible();
110 -});
\ No newline at end of file
 119+$( function(){
 120+ $( '.kr-collapsible' ).makeCollapsible();
 121+} );
\ No newline at end of file

Follow-up revisions

RevisionCommit summaryAuthorDate
r78935Improving jquery.makeCollapsible:...krinkle23:34, 23 December 2010
r78944Fixing TODO from r78915...krinkle01:30, 24 December 2010
r78967Adding message for collapsible link in core for localization (Follow-up r7893...krinkle15:52, 24 December 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r78914adding jQuery.makeCollapsible pluginkrinkle19:25, 23 December 2010

Comments

#Comment by The Evil IP address (talk | contribs)   21:40, 23 December 2010

Can we use another message than MediaWiki:Hide and MediaWiki:Show? They're used in the recent changes and what links here, so it's a better idea to split it up into another message, for example "collapsible-hide" and "collapsible-show".

#Comment by Krinkle (talk | contribs)   23:36, 23 December 2010

Since "show" and "hide" are too general, I've modified the keys in r78935 to "collapsible-expand" and "collapsible-collapse".

Status & tagging log