r72182 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r72181‎ \| r72182 \| r72183 >
Date:	00:00, 2 September 2010
Author:	adam
Status:	deferred
Tags:
Comment:	ArticleAssessment js improvements. Fixed some bad code in the stars plugin and general improvements and further implimentations in the AA code itself
Modified paths:	/trunk/extensions/ArticleAssessmentPilot/ArticleAssessmentPilot.hooks.php (modified) (history) /trunk/extensions/ArticleAssessmentPilot/css/ArticleAssessment.css (modified) (history) /trunk/extensions/ArticleAssessmentPilot/js/ArticleAssessment.js (modified) (history) /trunk/extensions/ArticleAssessmentPilot/js/jquery.stars.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleAssessmentPilot/css/ArticleAssessment.css
—	—	@@ -108,13 +108,25 @@
109	109	.article-assessment-wrapper .article-assessment-submit {
110	110	text-align: right;
111	111	}
112		~~-.article-assessment-wrapper .article-assessment-stale-msg {~~
113		~~- background: #d6f3ff;~~
114		~~- border: 1px solid #5dc9f4;~~
	112	+.article-assessment-wrapper .article-assessment-flash {
115	113	float: left;
116	114	font-size: 11px;
117	115	padding: 1px 5px;
	116	+ width: 560px;
	117	+ text-align: left;
118	118	}
	119	+.article-assessment-wrapper .article-assessment-stale-msg {
	120	+ background: #d6f3ff;
	121	+ border: 2px solid #5dc9f4;
	122	+}
	123	+.article-assessment-wrapper .article-assessment-success-msg {
	124	+ background: #e8e8e8;
	125	+ border: 2px solid #006505;
	126	+}
	127	+.article-assessment-wrapper .article-assessment-error-msg {
	128	+ background: #e8e8e8;
	129	+ border: 2px solid #a91100;
	130	+}
119	131	.article-assessment-wrapper .article-assessment-rating-field-name {
120	132	float: left;
121	133	width: 90px;
Index: trunk/extensions/ArticleAssessmentPilot/ArticleAssessmentPilot.hooks.php
—	—	@@ -89,6 +89,8 @@
90	90	'articleassessment-rating-neutrality-tooltip',
91	91	'articleassessment-rating-completeness-tooltip',
92	92	'articleassessment-rating-readability-tooltip',
	93	+ 'articleassessment-error',
	94	+ 'articleassessment-thanks',
93	95	'articleassessment-articlerating',
94	96	'articleassessment-featurefeedback',
95	97	'articleassessment-noratings',
Index: trunk/extensions/ArticleAssessmentPilot/js/ArticleAssessment.js
—	—	@@ -2,7 +2,7 @@
3	3	$.ArticleAssessment = {
4	4	'config': {
5	5	'authtoken': '',
6		~~- 'userID': wgUserName,~~
	6	+ 'userID': '',
7	7	'pageID': wgArticleId,
8	8	'revID': wgCurRevisionId
9	9	},
—	—	@@ -66,8 +66,8 @@
67	67	var config = $.ArticleAssessment.config;
68	68	// if this is an anon user, get a unique identifier for them
69	69	// load up the stored ratings and update the markup if the cookie exists
70		~~- var cookieSettings = $.cookie( 'mwArticleAssessment' );~~
71		~~- if ( true \|\| typeof cookieSettings == 'undefined' ) {~~
	70	+ var userToken = $.cookie( 'mwArticleAssessmentUserToken' );
	71	+ if ( typeof userToken == 'undefined' \|\| userToken == null ) {
72	72	function randomString( string_length ) {
73	73	var chars = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXTZabcdefghiklmnopqrstuvwxyz";
74	74	var randomstring = '';
—	—	@@ -77,24 +77,22 @@
78	78	}
79	79	return randomstring;
80	80	}
81		~~- cookieSettings = {~~
82		~~- uid: randomString( 32 )~~
83		~~- };~~
84		~~- $.cookie( 'mwArticleAssessment', cookieSettings );~~
	81	+ userToken = randomString( 32 );
	82	+ $.cookie( 'mwArticleAssessmentUserToken', userToken );
85	83	}
86	84	if ( ! wgUserName ) {
87		~~- config.userID = cookieSettings.uid;~~
	85	+ config.userID = userToken;
88	86	}
89	87	// setup our markup using the template varibales in settings
90	88	var $output = $( settings.structureHTML
91	89	.replace( /\{INSTRUCTIONS\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-pleaserate') )
92		~~- .replace( /\{FEEDBACK\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-featurefeedback')~~
	90	+ .replace( /\{FEEDBACK\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-featurefeedback')
93	91	.replace( /\[\[([^\\|\]])\\|([^\\|\]])\]\]/, '<a href="' + wgArticlePath + '">$2</a>' ) )
94		~~- .replace( /\{YOURFEEDBACK\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-yourfeedback') )~~
95		~~- .replace( /\{ARTICLERATING\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-articlerating' ) )~~
96		~~- .replace( /\{RESULTSHIDE\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-results-hide' )~~
	92	+ .replace( /\{YOURFEEDBACK\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-yourfeedback') )
	93	+ .replace( /\{ARTICLERATING\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-articlerating' ) )
	94	+ .replace( /\{RESULTSHIDE\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-results-hide' )
97	95	.replace( /\[\[\\|([^\]]*)\]\]/, '<a href="#">$1</a>' ) )
98		~~- .replace( /\{RESULTSSHOW\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-results-show' )~~
	96	+ .replace( /\{RESULTSSHOW\}/g, $.ArticleAssessment.fn.getMsg('articleassessment-results-show' )
99	97	.replace( /\[\[\\|([^\]]*)\]\]/, '<a href="#">$1</a>' ) ) );
100	98	for( var field in settings.fieldMessages ) {
101	99	$output.find( '.article-assessment-rating-fields' )
—	—	@@ -146,9 +144,9 @@
147	145
148	146	// set the height of our smaller fieldset to match the taller
149	147	if( $( '#article-assessment-rate' ).height() > $( '#article-assessment-ratings' ).height() ) {
150		~~- $( '#article-assessment-ratings' ).css( 'minHeight', $( '#article-assessment-rate' ).height() );~~
	148	+ $( '#article-assessment-ratings' ).css( 'minHeight', $( '#article-assessment-rate' ).height() );
151	149	} else {
152		~~- $( '#article-assessment-rate' ).css( 'minHeight', $( '#article-assessment-ratings' ).height() );~~
	150	+ $( '#article-assessment-rate' ).css( 'minHeight', $( '#article-assessment-ratings' ).height() );
153	151	}
154	152	// attempt to fetch the ratings
155	153	$.ArticleAssessment.fn.getRatingData();
—	—	@@ -177,7 +175,7 @@
178	176	$( this )
179	177	.after( $( '<span class="rating-field-hint" />' )
180	178	.attr( 'original-title', $( this ).attr( 'original-title' ) )
181		~~- .tipsy( { gravity : 'se', opacity: '0.9', } ) );~~
	179	+ .tipsy( { gravity : 'se', opacity: '0.9', } ) );
182	180	} );
183	181	// bind submit event to the form
184	182	$( '#article-assessment' )
—	—	@@ -189,35 +187,39 @@
190	188	// Request the ratings data for the current article
191	189	'getRatingData': function() {
192	190	var config = $( '#article-assessment' ).data( 'articleAssessment-context' ).config;
	191	+ var requestData = {
	192	+ 'action': 'query',
	193	+ 'list': 'articleassessment',
	194	+ 'aarevid': config.revID,
	195	+ 'aapageid': config.pageID,
	196	+ 'aauserrating': 1,
	197	+ 'format': 'json'
	198	+ }
	199	+ if( config.userID.length == 32 ) {
	200	+ requestData.aaanontoken = config.userID;
	201	+ }
193	202	var request = $.ajax( {
194	203	url: wgScriptPath + '/api.php',
195		~~- data: {~~
196		~~- 'action': 'query',~~
197		~~- 'list': 'articleassessment',~~
198		~~- 'aarevid': config.revID,~~
199		~~- 'aapageid': config.pageID,~~
200		~~- 'aauserrating': 1,~~
201		~~- 'aauserid': config.userID,~~
202		~~- 'format': 'json'~~
203		~~- },~~
	204	+ data: requestData,
204	205	dataType: 'json',
205	206	success: function( data ) {
206	207	$.ArticleAssessment.fn.afterGetRatingData( data );
207	208	},
208		~~- error: function(XMLHttpRequest, textStatus, errorThrown) {~~
209		~~- // console.log(XMLHttpRequest, textStatus, errorThrown);~~
	209	+ error: function( XMLHttpRequest, textStatus, errorThrown ) {
	210	+ $.ArticleAssessment.fn.flashNotice( $.ArticleAssessment.fn.getMsg( 'articleassessment-error' ),
	211	+ { 'class': 'article-assessment-error-msg' } );
210	212	}
211	213	} );
212	214	},
213	215	'afterGetRatingData' : function( data ) {
214	216	var settings = $( '#article-assessment' ).data( 'articleAssessment-context' ).settings;
215	217	// add the correct data to the markup
216		~~- if( data.query.articleassessment.length > 0 ) {~~
	218	+ if( data.query.articleassessment && data.query.articleassessment.length > 0 ) {
217	219	for( rating in data.query.articleassessment[0].ratings) {
218	220	var rating = data.query.articleassessment[0].ratings[rating],
219	221	$rating = $( '#' + rating.ratingdesc ),
220	222	count = rating.count,
221		~~- total = rating.total / count,~~
	223	+ total = ( rating.total / count ).toFixed( 1 ),
222	224	label = $.ArticleAssessment.fn.getMsg( 'articleassessment-noratings', [total, count] );
223	225	$rating
224	226	.find( '.article-assessment-rating-field-value' )
—	—	@@ -231,7 +233,7 @@
232	234	}
233	235	}
234	236	// if the rating is stale, add the stale class
235		~~- if( data.query.articleassessment ) {~~
	237	+ if( data.query.articleassessment.stale ) {
236	238	// add the stale star class to each on star
237	239	$( '.ui-stars-star-on' )
238	240	.addClass( 'ui-stars-star-stale' );
—	—	@@ -240,8 +242,11 @@
241	243	.replace( /'''([^']*)'''/g, '<strong>$1</strong>' )
242	244	.replace( /''([^']*)''/g, '<em>$1</em>' );
243	245	$.ArticleAssessment.fn.flashNotice( msg, { 'class': 'article-assessment-stale-msg' } );
	246	+ } else {
	247	+ // if it's not a stale rating, we want to make the stars blue
	248	+ $( '.ui-stars-star-on' ).addClass( 'ui-stars-star-rated' );
244	249	}
245		~~- }~~
	250	+ }
246	251	// initialize the ratings
247	252	$( '.article-assessment-rating-field-value' ).each( function() {
248	253	$( this )
—	—	@@ -255,8 +260,9 @@
256	261	// clear out the stale message
257	262	$.ArticleAssessment.fn.flashNotice( );
258	263
259		~~- //lock the star inputs~~
260		-
	264	+ // lock the star inputs & submit
	265	+ $( '.rating-field' ).stars( 'disable' );
	266	+ $( '#article-assessment input' ).attr( "disabled", "disabled" );
261	267	// get our results for submitting
262	268	var results = {};
263	269	$( '.rating-field input' ).each( function() {
—	—	@@ -280,14 +286,24 @@
281	287	},
282	288	dataType: 'json',
283	289	success: function( data ) {
	290	+ // update the ratings
	291	+ $.ArticleAssessment.fn.getRatingData();
284	292	// set the stars to rated status
285		~~- $j('.ui-stars-star-on').addClass('ui-stars-star-rated');~~
286		~~- // unlock the stars~~
287		-
	293	+ $( '.ui-stars-star-on' ).addClass( 'ui-stars-star-rated' );
	294	+ // unlock the stars & submit
	295	+ $( '.rating-field' ).stars( 'enable' );
	296	+ $( '#article-assessment input:disabled' ).removeAttr( "disabled" );
288	297	// update the results
289	298
290	299	// show the results
291	300	$( '#article-assessment .article-assessment-show-ratings a' ).click();
	301	+ // say thank you
	302	+ $.ArticleAssessment.fn.flashNotice( $.ArticleAssessment.fn.getMsg( 'articleassessment-thanks' ),
	303	+ { 'class': 'article-assessment-success-msg' } );
	304	+ },
	305	+ error: function( XMLHttpRequest, textStatus, errorThrown ) {
	306	+ $.ArticleAssessment.fn.flashNotice( $.ArticleAssessment.fn.getMsg( 'articleassessment-error' ),
	307	+ { 'class': 'article-assessment-error-msg' } );
292	308	}
293	309	} );
294	310	},
Index: trunk/extensions/ArticleAssessmentPilot/js/jquery.stars.js
—	—	@@ -264,10 +264,8 @@
265	265	$.fn.stars = function ( ) {
266	266	// convert the arguments to an array
267	267	var args = Array.prototype.slice.call(arguments);
268		-
269	268	// default value to return -- overwritten by api calls
270	269	var out = $( this );
271		-
272	270	$( this ).each( function() {
273	271	// get the context if it's already been initialized
274	272	var context = $( this ).data( 'stars-context' );
—	—	@@ -287,9 +285,9 @@
288	286	$.stars.create.call( context );
289	287	} else if ( typeof args[0] == 'string' ) {
290	288	// API call
291		~~- var funcName = args.shift();~~
	289	+ var funcName = args[0];
292	290	// call the function, and if it returns something, store the output in our return var
293		~~- out = $.stars[funcName].call( context, args[0] ) \|\| out;~~
	291	+ out = $.stars[funcName].call( context, args.slice(1) ) \|\| out;
294	292	}
295	293	} else {
296	294	// initialize with the defaults

Status & tagging log

18:47, 13 September 2010 Catrope (talk | contribs) changed the status of r72182 [removed: new added: deferred]