"Krinkle" changed the status of MediaWiki.r105613 to "fixme" and commented it.
URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/105613#c27652
Old Status: ok
> New Status: fixme
Commit summary for MediaWiki.r105613:
created moodbar user email input / confirm templates and js to post to new api,
frontend followup to rr105385. added moodbar tool tip module, moodbar status
bubble icons
Krinkle's comment:
<pre>
+ 'moodbar-tooltip-title' => 'Are you sad, happy, or confused about
editing Wikipedia?',
+ 'moodbar-tooltip-subtitle' => 'Your feedback about editing Wikipedia
helps us make the site better.',
</pre>
Don't hardcode anything Wikipedia specific, or Wikimedia specific in general
(Wikimedia has other projects as well, not to mention use by third-parties).
Use <code><nowiki>{{SITENAME}}</nowiki></code>. This means in JavaScript it
will likely need to be replaced manually with a regexp as this is not done by
default yet. (a simple <code>.replace( new RegExp( $.escapeRE('{{SITENAME}}'),
'g' ), mw.config.get( 'wgSiteName' ) );</code> will do the trick.
<pre>
+ $( '#fbd-filters-type-praise, #fbd-filters-type-confusion,
#fbd-filters-type-issues' ).each( function() {
+ $(this).prop( 'checked', true);
+ });
</pre>
No need for <code>.each()</code>. All jQuery methods (except for very few)
always run on the entire jQuery collection. No need to create a new jQuery
collection for <code>this</code> and then run <code>.prop()</code>( when loops
over the single-item collection). The following is how jQuery prototype is
intended:
<pre>
$( '#fbd-filters-type-praise, #fbd-filters-type-confusion,
#fbd-filters-type-issues' ).prop( 'checked', true);
</pre>
<pre>
+ mb.ui.overlay.find(
'.mw-moodBar-emailSubmit').removeAttr('disabled');
+ } else {
+ mb.ui.overlay.find(
'.mw-moodBar-emailSubmit').attr({'disabled':'true'});
</pre>
Initial states are set by the "disabled" attribute, but the interactive state
should be triggered by manipulating the property directly, not the attribtue.
Use <code>.prop( 'disabled', false );</code> and <code>.prop( 'disabled', true
);</code> respectively (which sets <code>element.disabled = ..;</code>
property, as supposed <code>element.setAttribute( 'disabled', .. );</code>
<pre>
+++ trunk/extensions/MoodBar/MoodBar.php (revision 105613)
@@ -91,6 +91,21 @@
),
);
+$wgResourceModules['ext.moodBar.tooltip'] = $mbResourceTemplate + array(
+ 'dependencies' => array(
+ 'jquery.client',
+ ),
</pre>
jQuery.client doesn't appear to be used by <code>ext.moodBar.tooltip</code>.
Roan already touched this area:
<pre>
+++ trunk/extensions/MoodBar/modules/ext.moodBar/ext.moodBar.init.js
(revision 105613)
var mb = mw.moodBar = {
+ userData: {}, //set by getUserInfo
@@ -54,13 +56,40 @@
+ // Assign user props to mb.userData object.
+ mb.getUserInfo();
},
+ getUserInfo: function() {
+ var query = {
+ action: 'query',
+ meta: 'userinfo',
+ uiprop: 'email',
+ format: 'json'
+ };
+ $(document).ready( function() {
+ $.ajax( {
+ 'type': 'POST',
+ 'url': mw.util.wikiScript( 'api' ),
+ 'data': query,
+ 'success': function (data) {
+ mb.userData =
data.query.userinfo;
+ },
+ 'error': function( jqXHR, textStatus,
errorThrown ) {
+ mb.userData = null;
+ },
+ 'dataType': 'json'
+ } );
+ });
+
</pre>
ApiQueryUserInfo is't a POST module. and <code>userData</code> appears unused.
Wherever it would be used, it would probably risk being not defined yet. Since
getuserInfo is delayed asynchronously by both document-ready and the
AJAX-request, it's hard impossibly to reliably use
<code>mw.moodBar.userData</code>.
Should probably take a callback as argument and cache it in a local variable
for immediate callback-calling if already fetched.
_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
