r103057 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r103056‎ | r103057 | r103058 >
Date:22:40, 14 November 2011
Author:rmoen
Status:ok (Comments)
Tags:todo 
Comment:
Moodbar phase 1 bug fixes, fixes bug32108 and addresses others in FeedbackDashboard requirements
Modified paths:
  • /trunk/extensions/MoodBar/SpecialFeedbackDashboard.php (modified) (history)
  • /trunk/extensions/MoodBar/modules/ext.moodBar.dashboard/ext.moodBar.dashboard.css (modified) (history)
  • /trunk/extensions/MoodBar/modules/ext.moodBar/ext.moodBar.core.js (modified) (history)

Diff [purge]

Index: trunk/extensions/MoodBar/SpecialFeedbackDashboard.php
@@ -218,12 +218,14 @@
219219
220220 $userPageUrl = htmlspecialchars($user->getUserPage()->getLocalURL());
221221
 222+ $userLink = Linker::userLink( $user->getId(), $username );
 223+
222224 return <<<HTML
223225 <div class="fbd-item-userName">
224 - <a href="$userPageUrl" class="fbd-item-userLink">$username</a>
225 - <sup class="fbd-item-userLinks">
 226+ $userLink
 227+ <span class="fbd-item-userLinks">
226228 $links
227 - </sup>
 229+ </span>
228230 </div>
229231 HTML;
230232 }
Index: trunk/extensions/MoodBar/modules/ext.moodBar/ext.moodBar.core.js
@@ -111,7 +111,7 @@
112112 .localize()
113113 .click( function( e ) {
114114 var $el = $( this );
115 - mb.ui.overlay.find( '.mw-moodBar-formSubmit').removeAttr('disabled');
 115+ //mb.ui.overlay.find( '.mw-moodBar-formSubmit').removeAttr('disabled');
116116 mb.ui.overlay.find( '.mw-moodBar-formInput' ).focus();
117117 $mwMoodBarTypes.addClass( 'mw-moodBar-types-select' );
118118 mb.feedbackItem.type = $el.attr( 'rel' );
@@ -120,6 +120,7 @@
121121 .find( '.mw-moodBar-selected' )
122122 .not( $el )
123123 .removeClass( 'mw-moodBar-selected' );
 124+ mb.validate();
124125 } )
125126 .get( 0 )
126127 );
@@ -228,9 +229,18 @@
229230 .val( mw.msg( 'moodbar-form-submit' ) )
230231 .click( function() {
231232 mb.feedbackItem.comment = mb.ui.overlay.find( '.mw-moodBar-formInput' ).val();
232 - mb.swapContent( mb.tpl.loading );
233 - $.moodBar.submit( mb.feedbackItem );
 233+ if(mb.feedbackItem.comment){
 234+ mb.swapContent( mb.tpl.loading );
 235+ $.moodBar.submit( mb.feedbackItem );
 236+ }
234237 } )
 238+ .end()
 239+
 240+ // Keypress
 241+ .find( '#mw-moodBar-feedbackInput' )
 242+ .keyup( function(event) {
 243+ mb.validate();
 244+ })
235245 .end();
236246
237247 // Set up character counter
@@ -298,6 +308,14 @@
299309 mb.prepareUserinputContent( mb.ui.overlay );
300310 }
301311 return true;
 312+ },
 313+
 314+ validate: function() {
 315+ if( $( '#mw-moodBar-feedbackInput' ).val() !== "" && $( '.mw-moodBar-selected').length ) {
 316+ mb.ui.overlay.find( '.mw-moodBar-formSubmit').removeAttr('disabled');
 317+ } else {
 318+ mb.ui.overlay.find( '.mw-moodBar-formSubmit').attr({'disabled':'true'});
 319+ }
302320 }
303321 } );
304322
Index: trunk/extensions/MoodBar/modules/ext.moodBar.dashboard/ext.moodBar.dashboard.css
@@ -2,7 +2,7 @@
33
44 #fbd-filters {
55 position: absolute;
6 - width: 12em;
 6+ width: 14em;
77 }
88
99 #fbd-filters form {
@@ -82,7 +82,7 @@
8383 /* List */
8484
8585 #fbd-list {
86 - margin: 0 0 0 13em;
 86+ margin: 0px 20px 0px 15em;
8787 padding: 0;
8888 min-height: 20em;
8989 list-style: none;
@@ -173,6 +173,7 @@
174174 .fbd-item {
175175 border-bottom: solid 1px silver;
176176 position: relative;
 177+ margin-bottom:1em;
177178 }
178179
179180 .fbd-item-emoticon {
@@ -210,7 +211,7 @@
211212 }
212213
213214 .fbd-item-userLinks {
214 - font-size: 0.5em;
 215+ font-size: 1em;
215216 }
216217
217218 .fbd-item-time {

Follow-up revisions

RevisionCommit summaryAuthorDate
r103060Bug 32108 - Adding server side check for empty commentbsitu22:51, 14 November 2011
r103061this fixes bug32108 - Adding server side check for empty commentbsitu22:57, 14 November 2011

Comments

#Comment by Raindrift (talk | contribs)   00:34, 16 November 2011
//mb.ui.overlay.find( '.mw-moodBar-formSubmit').removeAttr('disabled');

If you're going to remove code, remove it. Don't commit commented-out code. That's what VC is for.  :) Tagging this rev 'todo' for this.

if(mb.feedbackItem.comment){

Remember to use the "spacey" style. Sorry to be a pedant about it.

Lastly, when working with the keyup event, you may want to introduce a delay into your validation if it's complex. Probably not necessary here at the moment, but for an example of what I'm talking about, check out http://svn.wikimedia.org/svnroot/mediawiki/trunk/extensions/UploadWizard/resources/mw.DestinationChecker.js (validation methods have a way of growing...)

Status & tagging log