r98116 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r98115‎ | r98116 | r98117 >
Date:08:29, 26 September 2011
Author:catrope
Status:resolved (Comments)
Tags:
Comment:
MoodBar: Implement AJAX filtering in Special:MoodBarFeedback
* Factor out comment loading code into loadComments(), and allow it to operate in two modes
* Factor out display of errors/messages into showMessage()
* When viewing a permalink, load form state from cookies but don't act on it. If filtering happened on the PHP side, leave the form state alone completely. To determine which state we're in, expose data-filtertype on the <form> element from PHP.
* If there are no more results when filtering in PHP, hide the More link rather than not outputting it at all. We may need it later.
Modified paths:
  • /trunk/extensions/MoodBar/MoodBar.php (modified) (history)
  • /trunk/extensions/MoodBar/SpecialMoodBarFeedback.php (modified) (history)
  • /trunk/extensions/MoodBar/modules/ext.moodBar.dashboard/ext.moodBar.dashboard.js (modified) (history)

Diff [purge]

Index: trunk/extensions/MoodBar/SpecialMoodBarFeedback.php
——@@ -14,9 +14,11 @@
1515
1616 $limit = 20;
1717 $offset = false;
 18+ $filterType = '';
1819 $id = intval( $par );
1920 if ( $id > 0 ) {
2021 $filters = array( 'id' => $id );
 22+ $filterType = 'id';
2123 } else {
2224 // Determine filters and offset from the query string
2325 $filters = array();
——@@ -29,6 +31,7 @@
3032 $filters['username'] = $username;
3133 }
3234 $offset = $wgRequest->getVal( 'offset', $offset );
 35+ $filterType = 'filtered';
3336 }
3437 // Do the query
3538 $backwards = $wgRequest->getVal( 'dir' ) === 'prev';
——@@ -36,13 +39,13 @@
3740
3841 // Output HTML
3942 $wgOut->setPageTitle( wfMsg( 'moodbar-feedback-title' ) );
40 - $wgOut->addHTML( $this->buildForm() );
 43+ $wgOut->addHTML( $this->buildForm( $filterType ) );
4144 $wgOut->addHTML( $this->buildList( $res ) );
4245 $wgOut->addModuleStyles( 'ext.moodBar.dashboard.styles' );
4346 $wgOut->addModules( 'ext.moodBar.dashboard' );
4447 }
4548
46 - public function buildForm() {
 49+ public function buildForm( $filterType ) {
4750 global $wgRequest, $wgMoodBarConfig;
4851 $filtersMsg = wfMessage( 'moodbar-feedback-filters' )->escaped();
4952 $typeMsg = wfMessage( 'moodbar-feedback-filters-type' )->escaped();
——@@ -64,10 +67,11 @@
6568 array( 'id' => 'fbd-filters-type-issues', 'value' => 'sad' ) );
6669 $usernameTextbox = Html::input( 'username', $wgRequest->getText( 'username' ), 'text',
6770 array( 'id' => 'fbd-filters-username', 'class' => 'fbd-filters-input' ) );
 71+ $filterType = htmlspecialchars( $filterType );
6872
6973 return <<<HTML
7074 <div id="fbd-filters">
71 - <form action="$actionURL">
 75+ <form action="$actionURL" data-filtertype="$filterType">
7276 <h3 id="fbd-filters-title">$filtersMsg</h3>
7377 <fieldset id="fbd-filters-types">
7478 <legend class="fbd-filters-label">$typeMsg</legend>
——@@ -143,10 +147,13 @@
144148 $olderRow = $res['olderRow'];
145149 $newerRow = $res['newerRow'];
146150 $html = "<ul id=\"fbd-list\">$list</ul>";
147 - if ( $olderRow ) {
148 - $moreText = wfMessage( 'moodbar-feedback-more' )->escaped();
149 - $html .= '<div id="fbd-list-more"><a href="#">' . $moreText . '</a></div>';
 151+
 152+ $moreText = wfMessage( 'moodbar-feedback-more' )->escaped();
 153+ $attribs = array( 'id' => 'fbd-list-more' );
 154+ if ( !$olderRow ) {
 155+ $attribs['style'] = 'display: none;';
150156 }
 157+ $html .= Html::rawElement( 'div', $attribs, '<a href="#">' . $moreText . '</a></div>' );
151158
152159 $olderURL = $newerURL = false;
153160 if ( $olderRow ) {
Index: trunk/extensions/MoodBar/modules/ext.moodBar.dashboard/ext.moodBar.dashboard.js
——@@ -16,11 +16,13 @@
1717
1818 function loadFromCookies() {
1919 var cookieTypes = $.cookie( 'moodbar-feedback-types' );
20 - $username = $( '#fbd-filters-username' );
 20+ $username = $( '#fbd-filters-username' ),
 21+ changed = false;
2122 if ( $username.val() == '' ) {
2223 var cookieUsername = $.cookie( 'moodbar-feedback-username' );
2324 if ( cookieUsername != '' ) {
2425 $username.val( cookieUsername );
 26+ changed = true;
2527 }
2628 }
2729
——@@ -29,27 +31,43 @@
3032 $( '#fbd-filters-type-praise, #fbd-filters-type-confusion, #fbd-filters-type-issues' ).each( function() {
3133 if ( !$(this).prop( 'checked' ) && cookieTypes.indexOf( '|' + $(this).val() ) != -1 ) {
3234 $(this).prop( 'checked', true );
 35+ changed = true;
3336 }
3437 } );
3538 }
 39+ return changed;
3640 }
3741
38 - $( '#fbd-filters-set' ).click( setCookies );
39 - loadFromCookies();
 42+ function showMessage( text ) {
 43+ $( '#fbd-list-more' )
 44+ .children( 'a' )
 45+ .hide()
 46+ .end()
 47+ .children( 'span' )
 48+ .remove() // Remove any previous messages
 49+ .end()
 50+ .append( $( '<span>' ).text( text ) );
 51+ }
4052
41 - $( '#fbd-list-more').children( 'a' ).click( function( e ) {
42 - e.preventDefault();
43 -
 53+ function loadComments( mode ) {
4454 var limit = 20,
4555 username = $( '#fbd-filters-username' ).val(),
4656 types = getSelectedTypes(),
4757 reqData;
4858
 59+ if ( mode == 'filter' ) {
 60+ $( '#fbd-list' ).empty();
 61+ }
4962 // Hide the "More" link and put in a spinner
5063 $( '#fbd-list-more' )
 64+ .show() // may have been output with display: none;
5165 .addClass( 'mw-ajax-loader' )
5266 .children( 'a' )
53 - .css( 'visibility', 'hidden' ); // using .hide() messes up the layout
 67+ .css( 'visibility', 'hidden' ) // using .hide() cuts off the spinner
 68+ .show() // showMessage() may have called .hide()
 69+ .end()
 70+ .children( 'span' )
 71+ .remove(); // Remove any message added by showMessage()
5472
5573 // Build the API request
5674 reqData = {
——@@ -57,9 +75,11 @@
5876 'list': 'moodbarcomments',
5977 'format': 'json',
6078 'mbcprop': 'formatted',
61 - 'mbclimit': limit + 2, // we drop the first and last result
62 - 'mbccontinue': $( '#fbd-list').find( 'li:last' ).data( 'mbccontinue' )
 79+ 'mbclimit': limit + 2 // we drop the first and last result
6380 };
 81+ if ( mode == 'more' ) {
 82+ reqData['mbccontinue'] = $( '#fbd-list').find( 'li:last' ).data( 'mbccontinue' );
 83+ }
6484 if ( types.length ) {
6585 reqData['mbctype'] = types.join( '|' );
6686 }
——@@ -74,10 +94,10 @@
7595 $( '#fbd-list-more' )
7696 .removeClass( 'mw-ajax-loader' )
7797 .children( 'a' )
78 - .css( 'visibility', 'visible' );
 98+ .css( 'visibility', 'visible' ); // Undo visibility: hidden;
7999
80100 if ( !data || !data.query || !data.query.moodbarcomments ) {
81 - $( '#fbd-list-more' ).text( mw.msg( 'moodbar-feedback-ajaxerror' ) );
 101+ showMessage( mw.msg( 'moodbar-feedback-ajaxerror' ) );
82102 return;
83103 }
84104
——@@ -86,11 +106,18 @@
87107 $ul = $( '#fbd-list' ),
88108 moreResults = false,
89109 i;
90 - if ( len > 0 ) {
91 - // Drop the first element because it duplicates the last shown one
92 - comments.shift();
93 - len--;
 110+ if ( len == 0 ) {
 111+ if ( mode == 'more' ) {
 112+ showMessage( mw.msg( 'moodbar-feedback-nomore' ) );
 113+ } else {
 114+ showMessage( mw.msg( 'moodbar-feedback-noresults' ) );
 115+ }
 116+ return;
94117 }
 118+
 119+ // Drop the first element because it duplicates the last shown one
 120+ comments.shift();
 121+ len--;
95122 if ( len > limit ) {
96123 // Drop any elements past the limit. We do know there are more results now
97124 len = limit;
——@@ -102,15 +129,36 @@
103130 }
104131
105132 if ( !moreResults ) {
106 - $( '#fbd-list-more' ).text( mw.msg( 'moodbar-feedback-nomore' ) );
 133+ if ( mode == 'more' ) {
 134+ showMessage( mw.msg( 'moodbar-feedback-nomore' ) );
 135+ } else {
 136+ $( '#fbd-list-more' ).hide();
 137+ }
107138 }
108139 },
109140 'error': function( jqXHR, textStatus, errorThrown ) {
110 - $( '#fbd-list-more' )
111 - .removeClass( 'mw-ajax-loader' )
112 - .text( mw.msg( 'moodbar-feedback-ajaxerror' ) );
 141+ $( '#fbd-list-more' ).removeClass( 'mw-ajax-loader' );
 142+ showMessage( mw.msg( 'moodbar-feedback-ajaxerror' ) );
113143 },
114144 'dataType': 'json'
115145 } );
 146+ }
 147+
 148+ $( '#fbd-filters' ).children( 'form' ).submit( function( e ) {
 149+ e.preventDefault();
 150+ setCookies();
 151+ loadComments( 'filter' );
116152 } );
 153+
 154+ $( '#fbd-list-more' ).children( 'a' ).click( function( e ) {
 155+ e.preventDefault();
 156+ loadComments( 'more' );
 157+ } );
 158+
 159+ var filterType = $( '#fbd-filters' ).children( 'form' ).data( 'filtertype' );
 160+ if ( filterType != 'filtered' ) {
 161+ if ( loadFromCookies() && filterType != 'id' ) {
 162+ loadComments( 'filter' );
 163+ }
 164+ }
117165 } );
Index: trunk/extensions/MoodBar/MoodBar.php
——@@ -119,7 +119,11 @@
120120 $wgResourceModules['ext.moodBar.dashboard'] = $mbResourceTemplate + array(
121121 'scripts' => 'ext.moodBar.dashboard/ext.moodBar.dashboard.js',
122122 'dependencies' => array( 'mediawiki.util' ),
123 - 'messages' => array( 'moodbar-feedback-nomore', 'moodbar-feedback-ajaxerror' ),
 123+ 'messages' => array(
 124+ 'moodbar-feedback-nomore',
 125+ 'moodbar-feedback-noresults',
 126+ 'moodbar-feedback-ajaxerror'
 127+ ),
124128 );
125129
126130 $wgResourceModules['ext.moodBar.dashboard.styles'] = $mbResourceTemplate + array(

Follow-up revisions

RevisionCommit summaryAuthorDate
r98119Followup r98116: don't set $filterState = 'filtered'; if no filters were appliedcatrope09:14, 26 September 2011
r98126Fix typo in r98116 that resulted in misnested divscatrope12:16, 26 September 2011
r98300Fix stupid typo in r98080, r98116catrope10:55, 28 September 2011

Comments

#Comment by Krinkle (talk | contribs)   01:53, 28 September 2011 
-			$username = $( '#fbd-filters-username' );
+			$username = $( '#fbd-filters-username' ),
+			changed = false;

Originally by r98080, these are outside the var statement, making them implied global.

Status & tagging log