r81879 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r81878‎ \| r81879 \| r81880 >
Date:	09:35, 10 February 2011
Author:	aaron
Status:	deferred
Tags:
Comment:	* Converted JS/CSS loading and localization with JS to use ResourceLoader * Call updateTooltipAccessKeys() in updateSaveButton() JS to fix accesskey * Fixed credits (his name should only be on review.js)
Modified paths:	/trunk/extensions/FlaggedRevs/FlaggedRevs.hooks.php (modified) (history) /trunk/extensions/FlaggedRevs/FlaggedRevs.php (modified) (history) /trunk/extensions/FlaggedRevs/client/flaggedrevs.css (modified) (history) /trunk/extensions/FlaggedRevs/client/flaggedrevs.js (modified) (history) /trunk/extensions/FlaggedRevs/client/review.js (modified) (history)

Diff [purge]

Index: trunk/extensions/FlaggedRevs/FlaggedRevs.php
—	—	@@ -200,7 +200,7 @@
201	201	$wgAddGroups['bureaucrat'][] = 'reviewer';
202	202	$wgRemoveGroups['bureaucrat'][] = 'reviewer';
203	203
204		~~-# URL location for flaggedrevs.css and flaggedrevs.js~~
	204	+# URL location that has an /img subdirectory containing UI icons
205	205	# Use a literal $wgScriptPath as a placeholder for the runtime value of $wgScriptPath
206	206	$wgFlaggedRevsStylePath = '$wgScriptPath/extensions/FlaggedRevs/client';
207	207
—	—	@@ -245,9 +245,6 @@
246	246	$wgAvailableRights[] = 'movestable';
247	247	$wgAvailableRights[] = 'stablesettings';
248	248
249		~~-# Bump this number every time you change flaggedrevs.css/flaggedrevs.js~~
250		~~-$wgFlaggedRevStyleVersion = 85;~~
251		-
252	249	$wgExtensionFunctions[] = 'efLoadFlaggedRevs';
253	250
254	251	$dir = dirname( __FILE__ ) . '/';
—	—	@@ -356,6 +353,32 @@
357	354	$wgDefaultUserOptions['flaggedrevseditdiffs'] = true;
358	355	$wgDefaultUserOptions['flaggedrevsviewdiffs'] = false;
359	356
	357	+# JS/CSS modules and message bundles used by JS scripts
	358	+$dir = dirname( __FILE__ ) . '/client/';
	359	+$wgResourceModules['ext.flaggedRevs.basic'] = array(
	360	+ 'styles' => array( 'flaggedrevs.css' ),
	361	+ 'scripts' => array( 'flaggedrevs.js' ),
	362	+ 'messages' => array(
	363	+ 'revreview-toggle-show', 'revreview-toggle-hide',
	364	+ 'revreview-diff-toggle-show', 'revreview-diff-toggle-hide',
	365	+ 'revreview-log-toggle-show', 'revreview-log-toggle-hide',
	366	+ 'revreview-log-details-show', 'revreview-log-details-hide'
	367	+ ),
	368	+ 'dependencies' => array( 'jquery', 'mediawiki.legacy.wikibits' ),
	369	+ 'localBasePath' => $dir,
	370	+ 'remoteExtPath' => 'FlaggedRevs'
	371	+);
	372	+$wgResourceModules['ext.flaggedRevs.review'] = array(
	373	+ 'scripts' => array( 'review.js' ),
	374	+ 'messages' => array(
	375	+ 'savearticle', 'tooltip-save', 'accesskey-save',
	376	+ 'revreview-submitedit', 'revreview-submitedit-title'
	377	+ ),
	378	+ 'dependencies' => array( 'ext.flaggedRevs.basic', 'mediawiki.legacy.ajax' ),
	379	+ 'localBasePath' => $dir,
	380	+ 'remoteExtPath' => 'FlaggedRevs'
	381	+);
	382	+
360	383	# ####### HOOK TRIGGERED FUNCTIONS #########
361	384
362	385	# ######## User interface #########
Index: trunk/extensions/FlaggedRevs/FlaggedRevs.hooks.php
—	—	@@ -44,10 +44,10 @@
45	45	* Add FlaggedRevs css/js.
46	46	*/
47	47	protected static function injectStyleAndJS() {
48		~~- global $wgOut, $wgUser, $wgFlaggedRevStyleVersion;~~
	48	+ global $wgOut, $wgUser;
49	49	static $loadedModules = false;
50	50	if ( $loadedModules ) {
51		~~- return true; # Don't double-load~~
	51	+ return true; // don't double-load
52	52	}
53	53	$loadedModules = true;
54	54	$fa = FlaggedArticleView::globalArticleInstance();
—	—	@@ -55,44 +55,12 @@
56	56	if ( !$fa \|\| !$fa->isReviewable() ) {
57	57	return true;
58	58	}
59		~~- $stylePath = FlaggedRevs::styleUrlPath();~~
60		~~- # Get JS/CSS file locations~~
61		~~- $encCssFile = htmlspecialchars( "$stylePath/flaggedrevs.css?$wgFlaggedRevStyleVersion" );~~
62		~~- $encJsFile = htmlspecialchars( "$stylePath/flaggedrevs.js?$wgFlaggedRevStyleVersion" );~~
63		~~- # Add CSS file~~
64		~~- $wgOut->addExtensionStyle( $encCssFile );~~
65		~~- # Add main JS file~~
66		~~- $wgOut->includeJQuery();~~
67		~~- $wgOut->addScriptFile( $encJsFile );~~
	59	+ # Add main CSS & JS files
	60	+ $wgOut->addModules( 'ext.flaggedRevs.basic' );
68	61	# Add review form JS for reviewers
69	62	if ( $wgUser->isAllowed( 'review' ) ) {
70		~~- $encJsFile = htmlspecialchars( "$stylePath/review.js?$wgFlaggedRevStyleVersion" );~~
71		~~- $wgOut->addScriptFile( $encJsFile );~~
	63	+ $wgOut->addModules( 'ext.flaggedRevs.review' );
72	64	}
73		~~- # Set basic messages for all users...~~
74		~~- $msgs = array(~~
75		~~- 'diffToggleShow' => wfMsgHtml( 'revreview-diff-toggle-show' ),~~
76		~~- 'diffToggleHide' => wfMsgHtml( 'revreview-diff-toggle-hide' ),~~
77		~~- 'logToggleShow' => wfMsgHtml( 'revreview-log-toggle-show' ),~~
78		~~- 'logToggleHide' => wfMsgHtml( 'revreview-log-toggle-hide' ),~~
79		~~- 'logDetailsShow' => wfMsgHtml( 'revreview-log-details-show' ),~~
80		~~- 'logDetailsHide' => wfMsgHtml( 'revreview-log-details-hide' ),~~
81		~~- 'toggleShow' => wfMsgHtml( 'revreview-toggle-show' ),~~
82		~~- 'toggleHide' => wfMsgHtml( 'revreview-toggle-hide' )~~
83		~~- );~~
84		~~- # Extra reviewer messages...~~
85		~~- if ( $wgUser->isAllowed( 'review' ) ) {~~
86		~~- $msgs['saveArticle'] = wfMsgHtml( 'savearticle' );~~
87		~~- $msgs['tooltipSave'] = wfMsgHtml( 'tooltip-save' ) .~~
88		~~- ' [' . wfMsgHtml( 'accesskey-save' ) . ']';~~
89		~~- $msgs['submitArticle'] = wfMsg( 'revreview-submitedit' );~~
90		~~- $msgs['tooltipSubmit'] = wfMsg( 'revreview-submitedit-title' ) .~~
91		~~- ' ['. wfMsg( 'accesskey-save' ) . ']';~~
92		~~- }~~
93		~~- # Add msgs to JS~~
94		~~- $wgOut->addInlineScript(~~
95		~~- "FlaggedRevs.messages = " . Xml::encodeJsVar( (object)$msgs ) . ";" );~~
96		-
97	65	return true;
98	66	}
99	67
—	—	@@ -145,12 +113,7 @@
146	114	'Watchlist', 'Recentchanges', 'Contributions', 'Recentchangeslinked' );
147	115	foreach ( $spPages as $key ) {
148	116	if ( $title->isSpecial( $key ) ) {
149		~~- global $wgScriptPath, $wgFlaggedRevsStylePath, $wgFlaggedRevStyleVersion;~~
150		~~- $stylePath = str_replace( '$wgScriptPath',~~
151		~~- $wgScriptPath, $wgFlaggedRevsStylePath );~~
152		~~- $encCssFile = htmlspecialchars( "$stylePath/flaggedrevs.css?" .~~
153		~~- $wgFlaggedRevStyleVersion );~~
154		~~- $out->addExtensionStyle( $encCssFile );~~
	117	+ $out->addModuleStyles( 'ext.flaggedRevs.basic' ); // CSS only
155	118	break;
156	119	}
157	120	}
Index: trunk/extensions/FlaggedRevs/client/flaggedrevs.css
—	—	@@ -1,4 +1,4 @@
2		~~-/* Every time you change this CSS please bump $wgFlaggedRevStyleVersion in FlaggedRevs.php */~~
	2	+/* -- (c) Aaron Schulz */
3	3
4	4	/* Standard User Interface */
5	5	div.flaggedrevs_basic,
Index: trunk/extensions/FlaggedRevs/client/flaggedrevs.js
—	—	@@ -1,21 +1,10 @@
2		~~-/* -- (c) Aaron Schulz, Daniel Arnold 2008 */~~
	2	+/* -- (c) Aaron Schulz */
3	3
4		~~-/* Every time you change this JS please bump $wgFlaggedRevStyleVersion in FlaggedRevs.php */~~
5		-
6		~~-var FlaggedRevs = {~~
7		~~- 'messages': {~~
8		~~- 'diffToggleShow' : '(show changes)',~~
9		~~- 'diffToggleHide' : '(hide changes)',~~
10		~~- 'logToggleShow' : '(show log)',~~
11		~~- 'logToggleHide' : '(hide log)',~~
12		~~- 'logDetailsShow' : '(show details)',~~
13		~~- 'logDetailsHide' : '(hide details)',~~
14		~~- 'toggleShow' : '(+)',~~
15		~~- 'toggleHide' : '(-)'~~
16		~~- },~~
	4	+window.FlaggedRevs = {
17	5	/* Dropdown collapse timer */
18	6	'boxCollapseTimer': null,
19		~~- /* Hide rating/diff clutter */~~
	7	+
	8	+ /* Enables rating/diff clutter via show/hide */
20	9	'enableShowhide': function() {
21	10	// Rating detail box
22	11	var toggle = document.getElementById('mw-fr-revisiontoggle');
—	—	@@ -67,11 +56,11 @@
68	57	// Collapsed -> expand
69	58	if( ratings.style.display == 'none' ) {
70	59	this.showBoxDetails();
71		~~- toggle.innerHTML = this.messages.toggleHide;~~
	60	+ toggle.innerHTML = mediaWiki.msg('revreview-toggle-hide');
72	61	// Expanded -> collapse
73	62	} else {
74	63	this.hideBoxDetails();
75		~~- toggle.innerHTML = this.messages.toggleShow;~~
	64	+ toggle.innerHTML = mediaWiki.msg('revreview-toggle-show');
76	65	}
77	66	},
78	67
—	—	@@ -117,10 +106,12 @@
118	107	if( !toggle ) return;
119	108	if( diff.style.display == 'none' ) {
120	109	diff.style.display = 'block';
121		~~- toggle.getElementsByTagName('a')[0].innerHTML = this.messages.diffToggleHide;~~
	110	+ toggle.getElementsByTagName('a')[0].innerHTML =
	111	+ mediaWiki.msg('revreview-diff-toggle-hide');
122	112	} else {
123	113	diff.style.display = 'none';
124		~~- toggle.getElementsByTagName('a')[0].innerHTML = this.messages.diffToggleShow;~~
	114	+ toggle.getElementsByTagName('a')[0].innerHTML =
	115	+ mediaWiki.msg('revreview-diff-toggle-show');
125	116	}
126	117	},
127	118
—	—	@@ -132,10 +123,12 @@
133	124	if( !toggle ) return;
134	125	if( log.style.display == 'none' ) {
135	126	log.style.display = 'block';
136		~~- toggle.getElementsByTagName('a')[0].innerHTML = this.messages.logToggleHide;~~
	127	+ toggle.getElementsByTagName('a')[0].innerHTML =
	128	+ mediaWiki.msg('revreview-log-toggle-hide');
137	129	} else {
138	130	log.style.display = 'none';
139		~~- toggle.getElementsByTagName('a')[0].innerHTML = this.messages.logToggleShow;~~
	131	+ toggle.getElementsByTagName('a')[0].innerHTML =
	132	+ mediaWiki.msg('revreview-log-toggle-show');
140	133	}
141	134	},
142	135
—	—	@@ -147,37 +140,45 @@
148	141	if( !toggle ) return;
149	142	if( log.style.display == 'none' ) {
150	143	log.style.display = 'block';
151		~~- toggle.getElementsByTagName('a')[0].innerHTML = this.messages.logDetailsHide;~~
	144	+ toggle.getElementsByTagName('a')[0].innerHTML =
	145	+ mediaWiki.msg('revreview-log-details-hide');
152	146	} else {
153	147	log.style.display = 'none';
154		~~- toggle.getElementsByTagName('a')[0].innerHTML = this.messages.logDetailsShow;~~
	148	+ toggle.getElementsByTagName('a')[0].innerHTML =
	149	+ mediaWiki.msg('revreview-log-details-show');
155	150	}
156		~~- }~~
157		~~-};~~
158		-
159		~~-FlaggedRevs.setCheckTrigger = function() {~~
160		~~- var checkbox = document.getElementById("wpReviewEdit");~~
161		~~- if( checkbox ) {~~
162		~~- checkbox.onclick = FlaggedRevs.updateSaveButton;~~
163		~~- }~~
164		~~-};~~
165		-
166		~~-FlaggedRevs.updateSaveButton = function() {~~
167		~~- var checkbox = document.getElementById("wpReviewEdit");~~
168		~~- var save = document.getElementById("wpSave");~~
169		~~- if( checkbox && save ) {~~
170		~~- // Review pending changes~~
171		~~- if ( checkbox.checked ) {~~
172		~~- save.value = FlaggedRevs.messages.saveArticle;~~
173		~~- save.title = FlaggedRevs.messages.tooltipSave;~~
174		~~- // Submit for review~~
175		~~- } else {~~
176		~~- save.value = FlaggedRevs.messages.submitArticle;~~
177		~~- save.title = FlaggedRevs.messages.tooltipSubmit;~~
	151	+ },
	152	+
	153	+ /* Enables changing of save button when "review this" checkbox changes */
	154	+ 'setCheckTrigger': function() {
	155	+ var checkbox = document.getElementById("wpReviewEdit");
	156	+ if( checkbox ) {
	157	+ checkbox.onclick = FlaggedRevs.updateSaveButton;
178	158	}
	159	+ },
	160	+
	161	+ /* Update save button when "review this" checkbox changes */
	162	+ 'updateSaveButton': function() {
	163	+ var checkbox = document.getElementById("wpReviewEdit");
	164	+ var save = document.getElementById("wpSave");
	165	+ if( checkbox && save ) {
	166	+ // Review pending changes
	167	+ if ( checkbox.checked ) {
	168	+ save.value = mediaWiki.msg('savearticle');
	169	+ save.title = mediaWiki.msg('tooltip-save') +
	170	+ ' [' + mediaWiki.msg('accesskey-save') + ']';
	171	+ // Submit for review
	172	+ } else {
	173	+ save.value = mediaWiki.msg('revreview-submitedit');
	174	+ save.title = mediaWiki.msg('revreview-submitedit-title')
	175	+ + ' [' + mediaWiki.msg('accesskey-save') + ']';
	176	+ }
	177	+ }
	178	+ window.updateTooltipAccessKeys( [ save ] ); // update accesskey in save.title
179	179	}
180	180	};
181	181
	182	+// @TODO: move this crap to core or something
182	183	FlaggedRevs.getRevisionContents = function() {
183	184	//get the contents div and replace it with actual parsed article contents via an API call.
184	185	var contentsDiv = document.getElementById("mw-fr-revisioncontents");
—	—	@@ -188,7 +189,8 @@
189	190	var diffUIParams = document.getElementById("mw-fr-diff-dataform");
190	191	var oldRevId = diffUIParams.getElementsByTagName('input')[1].value;
191	192	var origContents = contentsDiv.innerHTML;
192		~~- contentsDiv.innerHTML = "<span class='loading mw-small-spinner spinner'></span><span class='loading' >" + wgRevContents.waiting + "</span>";~~
	193	+ contentsDiv.innerHTML = "<span class='loading mw-small-spinner spinner'>" +
	194	+ "</span><span class='loading' >" + wgRevContents.waiting + "</span>";
193	195	var requestArgs = 'action=parse&prop=text\|categorieshtml\|languageshtml&format=xml';
194	196	if ( window.wgCurRevisionId == oldRevId && window.wgPageName ) {
195	197	requestArgs += '&page=' + encodeURIComponent( window.wgPageName );
—	—	@@ -197,32 +199,33 @@
198	200	}
199	201
200	202	var call = jQuery.ajax({
201		~~- url : wgScriptPath + '/api.php',~~
202		~~- type : "GET",~~
203		~~- data : requestArgs,~~
204		~~- dataType: "xml",~~
205		~~- success : function( result ) {~~
206		~~- contentsDiv.innerHTML = "";~~
207		~~- contents = jQuery(result).find("text");~~
208		~~- if ( contents && contents.text() ) {~~
209		~~- contentsDiv.innerHTML += contents.text();~~
210		~~- } else {~~
211		~~- contentsDiv.innerHTML = wgRevContents.error + " " + origContents;~~
212		~~- }~~
213		~~- categoryhtml = jQuery(result).find("categorieshtml");~~
214		~~- if ( categoryhtml && categoryhtml.text() ) {~~
215		~~- contentsDiv.innerHTML += categoryhtml.text();~~
216		~~- }~~
217		~~- languageshtml = jQuery(result).find("languageshtml");~~
218		~~- if ( languageshtml && languageshtml.text() ) {~~
219		~~- contentsDiv.innerHTML += "<div class='langlinks' >" + languageshtml.text() + "</div>";~~
220		~~- }~~
221		-
222		~~- },~~
223		~~- error : function(xmlHttpRequest, textStatus, errThrown) {~~
	203	+ url : wgScriptPath + '/api.php',
	204	+ type : "GET",
	205	+ data : requestArgs,
	206	+ dataType: "xml",
	207	+ success : function( result ) {
	208	+ contentsDiv.innerHTML = "";
	209	+ contents = jQuery(result).find("text");
	210	+ if ( contents && contents.text() ) {
	211	+ contentsDiv.innerHTML += contents.text();
	212	+ } else {
224	213	contentsDiv.innerHTML = wgRevContents.error + " " + origContents;
225	214	}
226		~~- });~~
	215	+ categoryhtml = jQuery(result).find("categorieshtml");
	216	+ if ( categoryhtml && categoryhtml.text() ) {
	217	+ contentsDiv.innerHTML += categoryhtml.text();
	218	+ }
	219	+ languageshtml = jQuery(result).find("languageshtml");
	220	+ if ( languageshtml && languageshtml.text() ) {
	221	+ contentsDiv.innerHTML += "<div class='langlinks' >" +
	222	+ languageshtml.text() + "</div>";
	223	+ }
	224	+
	225	+ },
	226	+ error : function(xmlHttpRequest, textStatus, errThrown) {
	227	+ contentsDiv.innerHTML = wgRevContents.error + " " + origContents;
	228	+ }
	229	+ });
227	230	}
228	231	if ( prevLink ) {
229	232	prevLink.onclick = function() {
—	—	@@ -240,10 +243,7 @@
241	244	}
242	245	};
243	246
244		~~-FlaggedRevs.setJSTriggers = function() {~~
245		~~- FlaggedRevs.enableShowhide();~~
246		~~- FlaggedRevs.setCheckTrigger();~~
247		~~- FlaggedRevs.getRevisionContents();~~
248		~~-};~~
249		-
250		~~-window.onload = FlaggedRevs.setJSTriggers;~~
	247	+// Perform some onload (which is when this script is included) events:
	248	+FlaggedRevs.enableShowhide();
	249	+FlaggedRevs.setCheckTrigger();
	250	+FlaggedRevs.getRevisionContents();
Index: trunk/extensions/FlaggedRevs/client/review.js
—	—	@@ -1,8 +1,8 @@
2	2	/* -- (c) Aaron Schulz, Daniel Arnold 2008 */
3	3
4		~~-/* Every time you change this JS please bump $wgFlaggedRevStyleVersion in FlaggedRevs.php */~~
5		-
6	4	/*
	5	+* Updates for radios/checkboxes on patch by Daniel Arnold (bug 13744).
	6	+* Visually update the revision rating form on change.
7	7	* a) Disable submit in case of invalid input.
8	8	* b) Update colors when select changes (Opera already does this).
9	9	* c) Also remove comment box clutter in case of invalid input.
—	—	@@ -75,8 +75,6 @@
76	76	}
77	77	};
78	78
79		~~-hookEvent( "load", FlaggedRevs.maybeDisableAcceptButton );~~
80		-
81	79	FlaggedRevs.updateRatingFormColors = function() {
82	80	for( tag in wgFlaggedRevsParams.tags ) {
83	81	var controlName = "wp" + tag;
—	—	@@ -93,13 +91,12 @@
94	92	}
95	93	};
96	94
97		~~-hookEvent( "load", FlaggedRevs.updateRatingFormColors );~~
98		-
	95	+// @TODO: use jQuery AJAX
99	96	// dependencies:
100	97	// * ajax.js:
101	98	/extern sajax_init_object, sajax_do_call /
102	99	// * wikibits.js:
103		~~- /extern hookEvent, jsMsg /~~
	100	+ /extern jsMsg /
104	101	// These should have been initialized in the generated js
105	102	if( typeof wgAjaxReview === "undefined" \|\| !wgAjaxReview ) {
106	103	wgAjaxReview = {};
—	—	@@ -109,6 +106,7 @@
110	107	wgAjaxReview.inprogress = false; // ajax request in progress
111	108	wgAjaxReview.timeoutID = null; // see wgAjaxReview.ajaxCall
112	109
	110	+// Args build-up from radios/checkboxes based on patch by Daniel Arnold (bug 13744)
113	111	wgAjaxReview.ajaxCall = function() {
114	112	if( !wgAjaxReview.supported ) {
115	113	return true;
—	—	@@ -322,4 +320,7 @@
323	321	}
324	322	};
325	323
326		~~-hookEvent("load", wgAjaxReview.onLoad);~~
	324	+// Perform some onload (which is when this script is included) events:
	325	+FlaggedRevs.maybeDisableAcceptButton();
	326	+FlaggedRevs.updateRatingFormColors();
	327	+wgAjaxReview.onLoad();

Follow-up revisions

Revision	Commit summary	Author	Date
r82011	* Follow-up r81879: added single quotes to relative CSS url()...RL seems to n...	aaron	10:38, 12 February 2011

Status & tagging log

20:55, 15 April 2011 Reedy (talk | contribs) changed the status of r81879 [removed: new added: deferred]