r107305 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r107304‎ | r107305 | r107306 >
Date:23:19, 25 December 2011
Author:nikerabbit
Status:resolved (Comments)
Tags:
Comment:
Rework ext.translate.quickedit.js from global functions into mw.translate
Numerous JS fixes, like getting rid of legacy globals
Modified paths:
  • /trunk/extensions/Translate/resources/ext.translate.quickedit.js (modified) (history)
  • /trunk/extensions/Translate/utils/TranslationEditPage.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Translate/utils/TranslationEditPage.php
——@@ -194,7 +194,7 @@
195195
196196 return array(
197197 'onclick' => Xml::encodeJsCall(
198 - 'return trlOpenJsEdit', array( $title->getPrefixedDbKey(), $group ) ),
 198+ 'return mw.translate.openDialog', array( $title->getPrefixedDbKey(), $group ) ),
199199 'title' => wfMsg( 'translate-edit-title', $title->getPrefixedText() )
200200 );
201201 }
Index: trunk/extensions/Translate/resources/ext.translate.quickedit.js
——@@ -19,184 +19,193 @@
2020 * @license http://www.gnu.org/copyleft/gpl.html GNU General Public License 2.0 or later
2121 */
2222
23 -(function($) {
24 -
25 -function MessageCheckUpdater( callback ) {
 23+(function( $ ) {
 24+ "use strict";
2625
27 - this.act = function() {
28 - callback();
29 - delete this.timeoutID;
30 - };
 26+ var translate = {
 27+ dialogwidth: false,
 28+
 29+ init: function() {
 30+ mw.translate.dialogwidth = parseInt( mw.translate.windowWidth()*0.8, 10 );
 31+ },
3132
32 - this.setup = function() {
33 - this.cancel();
34 - var self = this;
35 - this.timeoutID = window.setTimeout( self.act, 1000 );
36 - };
 33+ MessageCheckUpdater: function( callback ) {
 34+ this.act = function() {
 35+ callback();
 36+ delete this.timeoutID;
 37+ };
3738
38 - this.cancel = function() {
39 - if ( typeof this.timeoutID === 'number' ) {
40 - window.clearTimeout( this.timeoutID );
41 - delete this.timeoutID;
42 - }
43 - };
44 -}
 39+ this.setup = function() {
 40+ this.cancel();
 41+ var self = this;
 42+ this.timeoutID = window.setTimeout( self.act, 1000 );
 43+ };
4544
46 -function trlVpWidth() {
47 - return window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth;
48 -}
 45+ this.cancel = function() {
 46+ if ( typeof this.timeoutID === 'number' ) {
 47+ window.clearTimeout( this.timeoutID );
 48+ delete this.timeoutID;
 49+ }
 50+ };
 51+ },
4952
50 -function addAccessKeys(dialog) {
51 - $( '[accesskey=a], [accesskey=s], [accesskey=d], [accesskey=h]' ).each(
52 - function( i ) {
53 - $(this).removeAttr( 'accesskey' );
54 - }
55 - );
56 - dialog.find( '.mw-translate-save' ).attr( 'accesskey', 'a' ).attr( 'title', '[' + tooltipAccessKeyPrefix + 'a]' );
57 - dialog.find( '.mw-translate-next' ).attr( 'accesskey', 's' ).attr( 'title', '[' + tooltipAccessKeyPrefix + 's]' );
58 - dialog.find( '.mw-translate-skip' ).attr( 'accesskey', 'd' ).attr( 'title', '[' + tooltipAccessKeyPrefix + 'd]' );
59 - dialog.find( '.mw-translate-history' ).attr( 'accesskey', 'h' ).attr( 'title', '[' + tooltipAccessKeyPrefix + 'h]' );
60 -}
 53+ windowWidth: function() {
 54+ return window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth;
 55+ },
6156
62 -var dialogwidth = false;
 57+ addAccessKeys: function( dialog ) {
 58+ var buttons = {
 59+ a: '.mw-translate-save',
 60+ s: '.mw-translate-next',
 61+ d: '.mw-translate-skip',
 62+ h: '.mw-translate-history'
 63+ };
6364
64 -window.trlOpenJsEdit = function( page, group ) {
65 - var url = wgScript + '?title=Special:Translate/editpage&suggestions=async&page=$1&loadgroup=$2';
66 - url = url.replace( '$1', encodeURIComponent( page ) ).replace( '$2', encodeURIComponent( group ) );
67 - var id = 'jsedit' + page.replace( /[^a-zA-Z0-9_]/g, '_' );
 65+ for ( var key in buttons ) {
 66+ if ( !buttons.hasOwnProperty( key ) ) {
 67+ continue;
 68+ }
6869
69 - var dialog = $( '#' + id );
70 - if ( dialog.size() > 0 ) {
71 - dialog.dialog( 'option', 'position', 'top' );
72 - dialog.dialog( 'open' );
73 - return false;
74 - }
 70+ $( buttons[key] )
 71+ .val( function( i, b ) { return b.replace( / \(.\)$/, '' ); } )
 72+ .removeAttr( 'accesskey' )
 73+ .attr( 'title', '' );
7574
76 - $( '<div/>' ).attr( 'id', id ).appendTo( $( 'body' ) );
77 - dialog = $( '#' + id );
 75+ dialog.find( buttons[key] )
 76+ .val( function( i, b ) { return b + ' (_)'.replace( '_', key ); } )
 77+ .attr( 'accesskey', key )
 78+ .attr( 'title', '[' + tooltipAccessKeyPrefix + key + ']' );
 79+ }
 80+ },
 81+
 82+ registerFeatures: function( dialog, form, page ) {
 83+ // Enable the collapsible element
 84+ var $identical = $( '.mw-identical-title' );
 85+ if ( $.isFunction( $identical.makeCollapsible ) ) {
 86+ $identical.makeCollapsible();
 87+ }
 88+
 89+ form.find( '.mw-translate-next' ).click( function() {
 90+ mw.translate.openNext( page );
 91+ } );
7892
79 - var spinner = $( '<div/>' ).attr( 'class', 'mw-ajax-loader' );
80 - dialog.html( $( '<div/>' ).attr( 'class', 'mw-ajax-dialog' ).html( spinner ) );
 93+ form.find( '.mw-translate-skip' ).click( function() {
 94+ mw.translate.openNext( page );
 95+ dialog.dialog( 'close' );
 96+ return false;
 97+ } );
8198
82 - dialog.load(url, false, function() {
83 - var form = $( '#' + id + ' form' );
 99+ form.find( '.mw-translate-history' ).click( function() {
 100+ window.open( mw.config.get( 'wgServer' ) + mw.config.get( 'wgScript' ) + '?action=history&title=' + form.find( 'input[name=title]' ).val() );
 101+ return false;
 102+ } );
 103+
 104+ form.find( '.mw-translate-support, .mw-translate-askpermission' ).click( function() {
 105+ // Can use .data() only with 1.4.3 or newer
 106+ window.open( $(this).attr( 'data-load-url' ) );
 107+ return false;
 108+ } );
84109
85 - form.hide().slideDown();
 110+ form.find( 'input#summary' ).focus( function() {
 111+ $(this).css( 'width', '85%' );
 112+ } );
 113+
 114+ var textarea = form.find( '.mw-translate-edit-area' );
 115+ textarea.css( 'display', 'block' );
 116+ textarea.autoResize( {extraSpace: 15, limit: 200} ).trigger( 'change' );
 117+ textarea.focus();
86118
87 - // Enable the collapsible element
88 - var $identical = $( '.mw-identical-title' );
89 - if ( $.isFunction( $identical.makeCollapsible ) ) {
90 - $identical.makeCollapsible();
91 - }
 119+ if ( form.find( '.mw-translate-messagechecks' ) ) {
 120+ var checker = new mw.translate.MessageCheckUpdater( function() {
 121+ var url = mw.config.get( 'wgScript' ) + '?title=Special:Translate/editpage&suggestions=checks&page=$1&loadgroup=$2';
 122+ url = url.replace( '$1', encodeURIComponent( page ) ).replace( '$2', encodeURIComponent( group ) );
 123+ $.post( url, { translation: textarea.val() }, function( mydata ) {
 124+ form.find( '.mw-translate-messagechecks' ).replaceWith( mydata );
 125+ } );
 126+ } );
92127
93 - form.find( '.mw-translate-next' ).click( function() {
94 - trlLoadNext( page );
95 - } );
 128+ textarea.keyup( function() { checker.setup() } );
 129+ }
 130+ },
96131
97 - form.find( '.mw-translate-skip' ).click( function() {
98 - trlLoadNext( page );
99 - dialog.dialog( 'close' );
100 - return false;
101 - } );
 132+ openDialog: function( page, group ) {
 133+ var url = mw.config.get( 'wgScript' ) + '?title=Special:Translate/editpage&suggestions=async&page=$1&loadgroup=$2';
 134+ url = url.replace( '$1', encodeURIComponent( page ) ).replace( '$2', encodeURIComponent( group ) );
 135+ var id = 'jsedit' + page.replace( /[^a-zA-Z0-9_]/g, '_' );
102136
103 - form.find( '.mw-translate-history' ).click( function() {
104 - window.open( wgServer + wgScript + '?action=history&title=' + form.find( 'input[name=title]' ).val() );
105 - return false;
106 - } );
107 -
108 - form.find( '.mw-translate-support,.mw-translate-askpermission' ).click( function() {
109 - // Can use .data() only with 1.4.3 or newer
110 - window.open( $(this).attr('data-load-url') );
111 - return false;
112 - } );
 137+ var dialog = $( '#' + id );
 138+ if ( dialog.size() > 0 ) {
 139+ dialog.dialog( 'option', 'position', 'top' );
 140+ dialog.dialog( 'open' );
 141+ return false;
 142+ }
113143
114 - form.find( 'input#summary' ).focus( function() {
115 - $(this).css('width', '85%');
116 - });
117 -
118 - var textarea = form.find( '.mw-translate-edit-area' );
119 - textarea.css( 'display', 'block' );
120 - textarea.autoResize({extraSpace: 15, limit: 200}).trigger( 'change' );
121 - textarea.focus();
 144+ $( '<div>' ).attr( 'id', id ).appendTo( $( 'body' ) );
 145+ dialog = $( '#' + id );
122146
123 - if ( form.find( '.mw-translate-messagechecks' ) ) {
124 - var checker = new MessageCheckUpdater( function() {
125 - var url = wgScript + '?title=Special:Translate/editpage&suggestions=checks&page=$1&loadgroup=$2';
126 - url = url.replace( '$1', encodeURIComponent( page ) ).replace( '$2', encodeURIComponent( group ) );
127 - $.post( url, { translation: textarea.val() }, function( mydata ) {
128 - form.find( '.mw-translate-messagechecks' ).replaceWith( mydata );
 147+ var spinner = $( '<div>' ).attr( 'class', 'mw-ajax-loader' );
 148+ dialog.html( $( '<div>' ).attr( 'class', 'mw-ajax-dialog' ).html( spinner ) );
 149+
 150+ dialog.load( url, false, function() {
 151+ var form = $( '#' + id + ' form' );
 152+
 153+ mw.translate.registerFeatures( dialog, form, page );
 154+ mw.translate.addAccessKeys( form );
 155+ form.hide().slideDown();
 156+
 157+ form.ajaxForm( {
 158+ dataType: 'json',
 159+ success: function(json) {
 160+ if ( json.error ) {
 161+ alert( json.error.info + ' (' + json.error.code +')' );
 162+ } else if ( json.edit.result === 'Failure' ) {
 163+ alert( mw.msg( 'translate-js-save-failed' ) );
 164+ } else if ( json.edit.result === 'Success' ) {
 165+ dialog.dialog( 'close' );
 166+ } else {
 167+ alert( mw.msg( 'translate-js-save-failed' ) );
 168+ }
 169+ }
129170 } );
130171 } );
131172
132 - textarea.keyup( function() {
133 - checker.setup();
 173+ dialog.dialog( {
 174+ bgiframe: true,
 175+ width: mw.translate.dialogwidth,
 176+ title: page,
 177+ position: 'top',
 178+ resize: function() { $( '#' + id + ' textarea' ).width( '100%' ); },
 179+ resizeStop: function() { mw.translate.dialogwidth = $( '#' + id ).width(); },
 180+ focus: function() { mw.translate.addAccessKeys( dialog ); },
 181+ close: function() { mw.translate.addAccessKeys( $([]) ); }
134182 } );
135 - }
136 -
137 - addAccessKeys( form );
138 - var b = null;
139 - b = form.find( '.mw-translate-save' );
140 - b.val( b.val() + ' (a)' );
141 - b = form.find( '.mw-translate-next' );
142 - b.val( b.val() + ' (s)' );
143 - b = form.find( '.mw-translate-skip' );
144 - b.val( b.val() + ' (d)' );
145 - b = form.find( '.mw-translate-history' );
146 - b.val( b.val() + ' (h)' );
147183
148 - form.ajaxForm( {
149 - dataType: 'json',
150 - success: function(json) {
151 - if ( json.error ) {
152 - alert( json.error.info + ' (' + json.error.code +')' );
153 - } else if ( json.edit.result === 'Failure' ) {
154 - alert( mw.msg( 'translate-js-save-failed' ) );
155 - } else if ( json.edit.result === 'Success' ) {
156 - dialog.dialog( 'close' );
157 - } else {
158 - alert( mw.msg( 'translate-js-save-failed' ) );
159 - }
160 - }
161 - } );
162 - } );
163 -
164 - dialog.dialog( {
165 - bgiframe: true,
166 - width: dialogwidth ? dialogwidth : parseInt( trlVpWidth()*0.8, 10 ),
167 - title: page,
168 - position: 'top',
169 - resize: function(event, ui) {
170 - $( '#' + id + ' textarea' ).width( '100%' );
 184+ return false;
171185 },
172 - resizeStop: function(event, ui) {
173 - dialogwidth = $( '#' + id ).width();
174 - },
175 - focus: function(event, ui) {
176 - addAccessKeys( dialog );
177 - },
178 - close: function(event, ui) {
179 - addAccessKeys( $([]) );
180 - }
181 - } );
182186
183 - return false;
184 -}
 187+ openNext: function( title ) {
 188+ var messages = mw.config.get( 'trlKeys' );
 189+ var page = title.replace( /[^:]+:/, '' );
 190+ var namespace = title.replace( /:.*/, '' );
 191+ var found = false;
 192+ for ( var key in messages ) {
 193+ if ( !messages.hasOwnProperty( key ) ) {
 194+ continue;
 195+ }
185196
186 -function trlLoadNext( title ) {
187 - var page = title.replace( /[^:]+:/, '' );
188 - var namespace = title.replace( /:.*/, '' );
189 - var found = false;
190 - for ( var key in trlKeys ) {
191 - if ( !trlKeys.hasOwnProperty(key) ) { continue; }
192 - var value = trlKeys[key];
193 - if (found) {
194 - return trlOpenJsEdit( namespace + ':' + value );
195 - } else if( page === value ) {
196 - found = true;
 197+ var value = messages[key];
 198+ if ( found ) {
 199+ return mw.translate.openDialog( namespace + ':' + value );
 200+ } else if( page === value ) {
 201+ found = true;
 202+ }
 203+ }
 204+ alert( mw.msg( 'translate-js-nonext' ) );
 205+ return;
197206 }
198 - }
199 - alert( mw.msg( 'translate-js-nonext' ) );
200 - return;
201 -}
 207+ };
 208+
 209+ mw.translate = translate;
 210+ $( document ).ready( mw.translate.init );
202211
203 -})(jQuery);
\ No newline at end of file
 212+} )( jQuery );
\ No newline at end of file

Follow-up revisions

RevisionCommit summaryAuthorDate
r107312More JS fixesnikerabbit00:19, 26 December 2011
r107753Trying to address my fixme in r107305 by hiding private stuffnikerabbit22:40, 31 December 2011

Comments

#Comment by Santhosh.thottingal (talk | contribs)   05:23, 27 December 2011

We are exposing these two things to public: mw.translate.dialogwidth and mw.translate.windowWidth() . Which I think, should be private.

#Comment by Nikerabbit (talk | contribs)   06:45, 28 December 2011

Can you tell/show how to do that?

#Comment by Santhosh.thottingal (talk | contribs)   05:37, 27 December 2011

In r107312, we used mw= mediaWiki and then at the end, mw.translate = translate; The code convention for extending mw is a bit different. See Manual:Coding_conventions/JavaScript#Closures

#Comment by Nikerabbit (talk | contribs)   06:47, 28 December 2011

I don't understand what you want me to do here? I don't see how the link is related to extending mw.

#Comment by Santhosh.thottingal (talk | contribs)   15:28, 28 December 2011

You may refer the webfonts extensions, ext.webfonts.js - Something like this: 

( function( $, mw, undefined ) {
mw.translate = {
// code goes here
}
})( jQuery, mediaWiki );
#Comment by Krinkle (talk | contribs)   20:25, 30 December 2011

@Santhosh: Are you referring to the way a reference to mw is established or the way the property is defined ?

Using var mw = mediaWiki; is indeed not the way we do it per conventions (there's nothing wrong with it, pure stylistic convention to use the closure-callee to alias it).

Using a local variable and exposing it at the end is fine, we do that a lot and is better for performance as well if you refer to the current object a lot.

Status & tagging log