r91374 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r91373‎ | r91374 | r91375 >
Date:15:28, 3 July 2011
Author:zhenya
Status:resolved (Comments)
Tags:
Comment:
Reworked UserStatus.js to be object-oriented
Modified paths:
  • /trunk/extensions/SocialProfile/SocialProfile.php (modified) (history)
  • /trunk/extensions/SocialProfile/UserStatus/UserStatus.js (modified) (history)
  • /trunk/extensions/SocialProfile/UserStatus/UserStatus_AjaxFunctions.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SocialProfile/SocialProfile.php
——@@ -94,7 +94,7 @@
9595 $wgFriendingEnabled = true;
9696
9797 // Should we enable UserStatus feature (currently is under development)
98 -$wgEnableUserStatus = false;
 98+$wgEnableUserStatus = true;
9999
100100 // Extension credits that show up on Special:Version
101101 $wgExtensionCredits['other'][] = array(
Index: trunk/extensions/SocialProfile/UserStatus/UserStatus_AjaxFunctions.php
——@@ -11,7 +11,7 @@
1212 $buf = $user_status_array['us_status'];
1313 $us = $buf;
1414 // @todo FIXME: i18n
15 - $us .= " <a href=\"javascript:toEditMode('$buf','$u_id');\">Edit</a>";
 15+ $us .= " <a href=\"javascript:UserStatus.toEditMode('$buf','$u_id');\">Edit</a>";
1616 return $us;
1717 }
1818
——@@ -24,7 +24,7 @@
2525 /*Under construction*/
2626 foreach ($historyArray as $row ) {
2727 $output .= '<tr><td id="status-history-time">'.$row['ush_timestamp'].' </td>';
28 - $output .= '<td><a href="javascript:fromHistoryToStatus(\''.$row['ush_status'].'\');">'
 28+ $output .= '<td><a href="javascript:UserStatus.fromHistoryToStatus(\''.$row['ush_status'].'\');">'
2929 .$row['ush_status'].'</a></td></tr>';
3030 }
3131 $output.='</table>';
Index: trunk/extensions/SocialProfile/UserStatus/UserStatus.js
——@@ -1,51 +1,52 @@
2 -var historyOpened = false;
 2+var UserStatus = {
 3+ historyOpened : false,
34
4 -function toShowMode( status, id ) {
5 - document.getElementById( 'user-status-block' ).innerHTML = status;
6 - document.getElementById( 'user-status-block' ).innerHTML += ' <a href="javascript:toEditMode(\'' + status + '\',' + id + ');">Edit</a>';
7 -}
 5+ toShowMode: function( status, id ) {
 6+ document.getElementById( 'user-status-block' ).innerHTML = status;
 7+ document.getElementById( 'user-status-block' ).innerHTML += ' <a href="javascript:UserStatus.toEditMode(\'' + status + '\',' + id + ');">Edit</a>';
 8+ },
 9+
 10+ toEditMode: function( status, id ) {
 11+ var editbar = '<input id="user-status-input" type="text" value="' + status + '">';
 12+ editbar += ' <a href="javascript:UserStatus.saveStatus(' + id + ');">Save</a>';
 13+ editbar += ' <a href="javascript:UserStatus.useHistory(' + id + ');">History</a>';
 14+ editbar += ' <a href="javascript:UserStatus.toShowMode(\'' + status + '\',' + id + ');">Cancel</a>';
 15+ document.getElementById( 'user-status-block' ).innerHTML = editbar;
 16+ },
 17+
 18+ saveStatus: function( id ) {
 19+ var div = document.getElementById( 'user-status-block' );
 20+ var ustext = document.getElementById( 'user-status-input' ).value;
 21+ sajax_do_call( 'wfSaveStatus', [id, ustext], div );
 22+ },
823
9 -function toEditMode( status, id ) {
10 - var editbar = '<input id="user-status-input" type="text" value="' + status + '">';
11 - editbar += ' <a href="javascript:saveStatus(' + id + ');">Save</a>';
12 - editbar += ' <a href="javascript:useHistory(' + id + ');">History</a>';
13 - editbar += ' <a href="javascript:toShowMode(\'' + status + '\',' + id + ');">Cancel</a>';
14 - document.getElementById( 'user-status-block' ).innerHTML = editbar;
15 -}
 24+ useHistory: function( id ){
 25+ if (this.historyOpened)
 26+ this.closeStatusHistory();
 27+ else
 28+ this.openStatusHistory(id);
 29+ },
1630
17 -function saveStatus( id ) {
18 - var div = document.getElementById( 'user-status-block' );
19 - var ustext = document.getElementById( 'user-status-input' ).value;
20 - sajax_do_call( 'wfSaveStatus', [id, ustext], div );
21 -}
 31+ openStatusHistory: function( id ) {
 32+ var historyBlock = document.getElementById('status-history-block');
 33+ if(historyBlock==null) {
 34+ var statusBlock = document.getElementById('user-status-block');
 35+ var historyBlock = document.createElement('div');
 36+ historyBlock.id = 'status-history-block';
 37+ statusBlock.appendChild(historyBlock);
 38+ }
 39+ this.historyOpened = true;
 40+ sajax_do_call( 'wfGetHistory', [id], historyBlock );
 41+ },
2242
23 -function useHistory(id){
24 - if (historyOpened){
25 - //alert('closed');
26 - closeStatusHistory();
27 - }
28 - else {
29 - //alert('opened');
30 - openStatusHistory(id);
31 - }
32 -}
 43+ closeStatusHistory: function() {
 44+ var hBlock = document.getElementById('status-history-block');
 45+ hBlock.parentNode.removeChild(hBlock);
 46+ this.historyOpened = false;
 47+ },
3348
34 -function openStatusHistory(id) {
35 - var statusBlock = document.getElementById('user-status-block');
36 - var historyBlock = document.createElement('div');
37 - historyBlock.id = 'status-history-block';
38 - statusBlock.appendChild(historyBlock);
39 - historyOpened = true;
40 - sajax_do_call( 'wfGetHistory', [id], historyBlock );
41 -}
42 -
43 -function closeStatusHistory() {
44 - var hBlock = document.getElementById('status-history-block');
45 - hBlock.parentNode.removeChild(hBlock);
46 - historyOpened = false;
47 -}
48 -
49 -function fromHistoryToStatus(str)
50 -{
51 - document.getElementById('user-status-input').value = str;
 49+ fromHistoryToStatus: function( str )
 50+ {
 51+ document.getElementById('user-status-input').value = str;
 52+ }
5253 }
\ No newline at end of file

Comments

#Comment by Jack Phoenix (talk | contribs)   16:57, 3 July 2011

Awesome! :D There are some minor coding style issues, but those can easily be fixed later on, either manually or via the stylize.php script. The actual JS rewrite looks OK to me, but JSHint suggests the following little tweaks:

  • wrap the if-else statement in UserStatus.useHistory inside braces
  • in UserStatus.openStatusHistory, use three equal signs to compare with null
  • in UserStatus.openStatusHistory, in the "if historyBlock is null" statement, don't prefix historyBlock with the var keyword since that variable is already declared
  • the last line, which closes the var UserStatus = { statement, should have a semicolon

Please revert the change you committed to SocialProfile.php, though — I don't think that UserStatus is ready for production just yet.

#Comment by Zhenya (talk | contribs)   17:01, 3 July 2011

understood. Will be done. About SocialProfile.php - forgot to switch it off after testing.

Status & tagging log