r113616 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r113615‎ \| r113616 \| r113617 >
Date:	12:14, 12 March 2012
Author:	nikerabbit
Status:	ok (Comments)
Tags:	i18nreview
Comment:	Improved and cleaned up Special:LanguageStats JavaScript to support collapsing/expanding nested subgroups
Modified paths:	/trunk/extensions/Translate/README (modified) (history) /trunk/extensions/Translate/resources/ext.translate.special.languagestats.css (modified) (history) /trunk/extensions/Translate/resources/ext.translate.special.languagestats.js (modified) (history) /trunk/extensions/Translate/specials/SpecialLanguageStats.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Translate/resources/ext.translate.special.languagestats.js
—	—	@@ -1,78 +1,104 @@
2	2	/**
3	3	* Collapsing script for Special:LanguageStats in MediaWiki Extension:Translate
4	4	* @author Krinkle <krinklemail (at) gmail (dot) com>
	5	+ * @author Niklas Laxström, 2012
5	6	* @created January 3, 2011
6	7	* @license GPL v2, CC-BY-SA-3.0
7	8	*/
	9	+/global mw:false/
8	10	jQuery( document ).ready( function( $ ) {
	11	+ "use strict";
9	12
10		~~- var $translateTable = $( '.mw-sp-translate-table' ),~~
11		~~- $metaRows = $( 'tr[data-ismeta=1]', $translateTable );~~
	13	+ var
	14	+ $translateTable = $( '.mw-sp-translate-table' ),
	15	+ $metaRows = $( 'tr.AggregateMessageGroup', $translateTable );
12	16
13		~~- // Only do stuff if there are any meta group rows on this pages~~
14		~~- if ( $metaRows.size() ) {~~
	17	+ // Quick return
	18	+ if ( !$metaRows.size() ) {
	19	+ return;
	20	+ }
15	21
16		~~- $metaRows.each( function() {~~
17		~~- // Get info and cache selectors~~
18		~~- var $thisGroup = $(this),~~
19		~~- thisGroupId = $thisGroup.attr( 'data-groupid' ),~~
20		~~- $thisChildRows = $( 'tr[data-parentgroups~="' + thisGroupId + '"]', $translateTable );~~
	22	+ $metaRows.each( function() {
	23	+ var
	24	+ $parent = $( this ),
	25	+ thisGroupId = $parent.attr( 'data-groupid' ),
	26	+ $children = $( 'tr[data-parentgroup="' + thisGroupId + '"]', $translateTable );
21	27
22		~~- // Only do the collapse stuff if this Meta-group actually has children on this page~~
23		~~- if ( $thisChildRows.size() ) {~~
	28	+ // Only do the collapse stuff if this Meta-group actually has children on this page
	29	+ if ( !$children.size() ) {
	30	+ return;
	31	+ }
24	32
25		~~- // Build toggle link~~
26		- var $toggler = $( '<span class="mw-sp-langstats-toggle mw-sp-langstats-expander">[</span>' ).append( $( '<a href="#" onclick="return false;"></a>' ).text( mw.msg( 'translate-langstats-expand' ) ) ).append( ']' ).click( function() {
27		~~- var $el = $( this );~~
28		~~- // Switch the state and toggle the rows~~
29		~~- if ( $el.hasClass( 'mw-sp-langstats-expander' ) ) {~~
30		~~- $thisChildRows.fadeIn();~~
31		~~- $el.removeClass( 'mw-sp-langstats-expander' ).addClass( 'mw-sp-langstats-collapser' )~~
32		~~- .find( '> a' ).text( mw.msg( 'translate-langstats-collapse' ) );~~
33		~~- } else {~~
34		~~- $thisChildRows.fadeOut();~~
35		~~- $el.addClass( 'mw-sp-langstats-expander' ).removeClass( 'mw-sp-langstats-collapser' )~~
36		~~- .find( '> a' ).text( mw.msg( 'translate-langstats-expand' ) );~~
37		~~- }~~
38		~~- } );~~
	33	+ // Build toggle link
	34	+ var $toggler = $( '<span class="groupexpander collapsed">[</span>' )
	35	+ .append( $( '<a href="#" onclick="return false;"></a>' )
	36	+ .text( mw.msg( 'translate-langstats-expand' ) ) )
	37	+ .append( ']' )
	38	+ .click( function() {
	39	+ var $el = $( this );
	40	+ // Switch the state and toggle the rows
	41	+ if ( $el.hasClass( 'collapsed' ) ) {
	42	+ $children.fadeIn().trigger( 'show' );
	43	+ $el.removeClass( 'collapsed' ).addClass( 'expanded' );
	44	+ $el.find( '> a' ).text( mw.msg( 'translate-langstats-collapse' ) );
	45	+ } else {
	46	+ $children.fadeOut().trigger( 'hide' );
	47	+ $el.addClass( 'collapsed' ).removeClass( 'expanded' );
	48	+ $el.find( '> a' ).text( mw.msg( 'translate-langstats-expand' ) );
	49	+ }
	50	+ } );
39	51
40		~~- // Add the toggle link to the first cell of the meta group table-row~~
41		~~- $thisGroup.find( ' > td:first' ).append( $toggler );~~
	52	+ // Add the toggle link to the first cell of the meta group table-row
	53	+ $parent.find( ' > td:first' ).append( $toggler );
	54	+
	55	+ // Handle hide/show recursively, so that collapsing parent group
	56	+ // hides all sub groups regardless of nesting level
	57	+ $parent.on( 'hide show', function( event ) {
	58	+ // Reuse $toggle, $parent and $children from parent scope
	59	+ if ( $toggler.hasClass( 'expanded' ) ) {
	60	+ $children.trigger( event.type )[event.type]();
42	61	}
43	62	} );
	63	+ } );
44	64
45		~~- // Create, bind and append the toggle-all button~~
46		~~- var $allChildRows = $( 'tr[data-parentgroups]', $translateTable ),~~
47		~~- $allToggles_cache = null,~~
48		- $toggleAllButton = $( '<span class="mw-sp-langstats-expander">[</span>' ).append( $( '<a href="#" onclick="return false;"></a>' ).text( mw.msg( 'translate-langstats-expandall' ) ) ).append( ']' ).click( function() {
49		~~- var $el = $( this ),~~
50		~~- $allToggles = !!$allToggles_cache ? $allToggles_cache : $( '.mw-sp-langstats-toggle', $translateTable );~~
	65	+ // Create, bind and append the toggle-all button
	66	+ var
	67	+ $allChildRows = $( 'tr[data-parentgroup]', $translateTable ),
	68	+ $allToggles_cache = null,
	69	+ $toggleAllButton = $( '<span class="collapsed">[</span>' )
	70	+ .append( $( '<a href="#" onclick="return false;"></a>' )
	71	+ .text( mw.msg( 'translate-langstats-expandall' ) ) )
	72	+ .append( ']' )
	73	+ .click( function() {
	74	+ var
	75	+ $el = $( this ),
	76	+ $allToggles = !!$allToggles_cache ? $allToggles_cache : $( '.groupexpander', $translateTable );
51	77	// Switch the state and toggle the rows
52	78	// and update the local toggles too
53		~~- if ( $el.hasClass( 'mw-sp-langstats-expander' ) ) {~~
	79	+ if ( $el.hasClass( 'collapsed' ) ) {
54	80	$allChildRows.show();
55		~~- $el.add( $allToggles ).removeClass( 'mw-sp-langstats-expander' ).addClass( 'mw-sp-langstats-collapser' );~~
	81	+ $el.add( $allToggles ).removeClass( 'collapsed' ).addClass( 'expanded' );
56	82	$el.find( '> a' ).text( mw.msg( 'translate-langstats-collapseall' ) );
57	83	$allToggles.find( '> a' ).text( mw.msg( 'translate-langstats-collapse' ) );
58	84	} else {
59	85	$allChildRows.hide();
60		~~- $el.add( $allToggles ).addClass( 'mw-sp-langstats-expander' ).removeClass( 'mw-sp-langstats-collapser' );~~
	86	+ $el.add( $allToggles ).addClass( 'collapsed' ).removeClass( 'expanded' );
61	87	$el.find( '> a' ).text( mw.msg( 'translate-langstats-expandall' ) );
62	88	$allToggles.find( '> a' ).text( mw.msg( 'translate-langstats-expand' ) );
63	89	}
64	90	} );
65	91
66		~~- // Initially hide them~~
67		~~- $allChildRows.hide();~~
	92	+ // Initially hide them
	93	+ $allChildRows.hide();
68	94
69		~~- // Add the toggle-all button above the table~~
70		~~- $( '<p class="mw-sp-langstats-toggleall"></p>' ).append( $toggleAllButton ).insertBefore( $translateTable );~~
71		~~- }~~
	95	+ // Add the toggle-all button above the table
	96	+ $( '<p class="groupexpander-all"></p>' ).append( $toggleAllButton ).insertBefore( $translateTable );
72	97	} );
73	98
74	99	// When hovering a row, adjust brightness of the last two custom-colored cells as well
75	100	// See also translate.langstats.css for the highlighting for the other normal rows
76	101	mw.loader.using( 'jquery.colorUtil' , function() {
	102	+ "use strict";
77	103	jQuery( document ).ready( function( $ ) {
78	104	$( '.mw-sp-translate-table.wikitable tr' ).hover( function() {
79	105	$( '> td.hover-color', this )
Index: trunk/extensions/Translate/resources/ext.translate.special.languagestats.css
—	—	@@ -2,20 +2,20 @@
3	3	background: white;
4	4	}
5	5
6		~~-.mw-sp-langstats-toggleall {~~
	6	+.groupexpander-all {
7	7	text-align: right;
8	8	}
9	9
10		~~-.mw-sp-langstats-toggle {~~
	10	+.groupexpander {
11	11	float: right;
12	12	}
13	13
14		~~-.mw-sp-langstats-collapser,~~
15		~~-.mw-sp-langstats-collapser a {~~
	14	+.expanded,
	15	+.expanded a {
16	16	cursor: n-resize;
17	17	}
18	18
19		~~-.mw-sp-langstats-expander,~~
20		~~-.mw-sp-langstats-expander a {~~
	19	+.collapsed,
	20	+.collapsed a {
21	21	cursor: s-resize;
22		-}
\ No newline at end of file
	22	+}
Index: trunk/extensions/Translate/README
—	—	@@ -30,6 +30,8 @@
31	31	https://translatewiki.net/docs/Translate/html/
32	32
33	33	== Change log ==
	34	+* 2012-03-12
	35	+* Special:LanguageStats group collapsing now supports nested subgroups
34	36	* 2012-03-11
35	37	- Support for shared TTMServer databases was added
36	38	- Suggestions from different TTMServers are now grouped
Index: trunk/extensions/Translate/specials/SpecialLanguageStats.php
—	—	@@ -327,35 +327,39 @@
328	328	}
329	329
330	330	/**
331		~~- * @param $item~~
332		~~- * @param $cache~~
333		~~- * @param $parent string~~
	331	+ * Creates a html table row for given (top-level) message group.
	332	+ * If $item is an array, meaning that the first group is an
	333	+ * AggregateMessageGroup and the latter are its children, it will recurse
	334	+ * and create rows for them too.
	335	+ * @param $item Array\|MessageGroup
	336	+ * @param $cache Array Cache as returned by MessageGroupStats::forLanguage
	337	+ * @param $parent MessageGroup (do not use, used internally only)
334	338	* @return string
335	339	*/
336		~~- protected function makeGroupGroup( $item, $cache, $parent = '' ) {~~
	340	+ protected function makeGroupGroup( $item, array $cache, MessageGroup $parent = null ) {
337	341	if ( !is_array( $item ) ) {
338		~~- return $this->makeGroupRow( $item, $cache, $parent === '' ? false : $parent );~~
	342	+ return $this->makeGroupRow( $item, $cache, $parent );
339	343	}
340	344
	345	+ // The first group in the array is the parent AggregateMessageGroup
341	346	$out = '';
342	347	$top = array_shift( $item );
343		~~- $out .= $this->makeGroupRow( $top, $cache, $parent === '' ? true : $parent );~~
	348	+ $out .= $this->makeGroupRow( $top, $cache, $parent );
344	349
	350	+ // Rest are children
345	351	foreach ( $item as $subgroup ) {
346		~~- $parents = trim( $parent . ' ' . $top->getId() );~~
347		~~- $out .= $this->makeGroupGroup( $subgroup, $cache, $parents );~~
	352	+ $out .= $this->makeGroupGroup( $subgroup, $cache, $top );
348	353	}
349	354
350	355	return $out;
351	356	}
352	357
353	358	/**
354		~~- * @param $group~~
355		~~- * @param $cache~~
356		~~- * @param $parent bool~~
	359	+ * Actually creates the table for single message group, unless it
	360	+ * is blacklisted or hidden by filters.
357	361	* @return string
358	362	*/
359		~~- protected function makeGroupRow( $group, $cache, $parent = false ) {~~
	363	+ protected function makeGroupRow( MessageGroup $group, array $cache, MessageGroup $parent = null ) {
360	364	$groupId = $group->getId();
361	365
362	366	if ( $this->table->isBlacklisted( $groupId, $this->target ) !== null ) {
—	—	@@ -394,11 +398,9 @@
395	399
396	400	$rowParams = array();
397	401	$rowParams['data-groupid'] = $groupId;
398		-
399		~~- if ( is_string( $parent ) ) {~~
400		~~- $rowParams['data-parentgroups'] = $parent;~~
401		~~- } elseif ( $parent === true ) {~~
402		~~- $rowParams['data-ismeta'] = '1';~~
	402	+ $rowParams['class'] = get_class( $group );
	403	+ if ( $parent ) {
	404	+ $rowParams['data-parentgroup'] = $parent->getId();
403	405	}
404	406
405	407	$out = "\t" . Html::openElement( 'tr', $rowParams );

Follow-up revisions

Revision	Commit summary	Author	Date
r114369	Get rid of onclick, ping comments in r113616...	nikerabbit	13:53, 21 March 2012

Comments

#Comment by Santhosh.thottingal (talk | contribs) 05:46, 14 March 2012

Found that the expansion does not work if the group name is non-English. See this expand/collapse link generated.

<a class="mw-sp-showmore" onclick="jQuery("#mw-subgroup-.E0.B4.9A.E0.B4.BF.E0.B4.A8.E0.B5.8D.E0.B4.A4.E0.B4.BE.E0.B4.AD.E0.B4.BE.E0.B4.B0.E0.B4.82").toggle()">Show the 3 subgroups.</a>

Nothing happens on click of this link, probably because of the periods in the Id that jquery cannot handle well.

#Comment by Krinkle (talk | contribs) 11:05, 14 March 2012

That's a problem indeed. However that wasn't introduced in this revision. It was done in r76102 and r72409.

Can be fixed by using jQuery( document.getElementById( 'mw-subgroup-.E0.B4.9A.E0' ) ); instead, or better.. don't use those ids in the first place but that may not be an immediate option.

#Comment by Krinkle (talk | contribs) 06:56, 14 March 2012

+		var $toggler = $( '<span class="groupexpander collapsed">[</span>' )
+			.append( $( '<a href="#" onclick="return false;"></a>' )
+			.text( mw.msg( 'translate-langstats-expand' ) ) )
+			.append( ']' )
+			.click( function() {

Use e.preventDefault() instead of using return false; via eval()'ed onclick attributes (which is a bit slower and creates a new function context as well).

.click( function ( e ) {
    /* code */
    e.preventDefault();

Status & tagging log

18:45, 21 March 2012 Catrope (talk | contribs) changed the tags for r113616 [removed: gerritmigration]
10:51, 21 March 2012 Siebrand (talk | contribs) changed the status of r113616 [removed: new added: ok]
01:03, 21 March 2012 Catrope (talk | contribs) changed the tags for r113616 [added: gerritmigration]
18:31, 13 March 2012 Siebrand (talk | contribs) changed the tags for r113616 [added: i18nreview]