r70529 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r70528‎ | r70529 | r70530 >
Date:23:24, 5 August 2010
Author:kaldari
Status:resolved (Comments)
Tags:
Comment:
some interface/localization clean-up for CentralNotice
Modified paths:
  • /trunk/extensions/CentralNotice/CentralNotice.i18n.php (modified) (history)
  • /trunk/extensions/CentralNotice/SpecialCentralNotice.php (modified) (history)
  • /trunk/extensions/CentralNotice/SpecialNoticeTemplate.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/SpecialCentralNotice.php
——@@ -1355,7 +1355,7 @@
13561356 );
13571357 $htmlOut .= Xml::tags( 'div',
13581358 array( 'style' => 'margin-top: 0.2em;' ),
1359 - '<img src="'.$scriptPath.'/arrow.png" style="vertical-align:baseline;"/>Select: <a href="#" onclick="selectLanguages(true);return false;">All</a>, <a href="#" onclick="selectLanguages(false);return false;">None</a>, <a href="#" onclick="top10Languages();return false;">Top 10 Languages</a>'
 1359+ '<img src="'.$scriptPath.'/arrow.png" style="vertical-align:baseline;"/>' . wfMsg( 'centralnotice-select' ) . ': <a href="#" onclick="selectLanguages(true);return false;">All</a>, <a href="#" onclick="selectLanguages(false);return false;">None</a>, <a href="#" onclick="top10Languages();return false;">Top 10 Languages</a>'
13601360 );
13611361 } else {
13621362 $htmlOut .= Xml::tags( 'select',
Index: trunk/extensions/CentralNotice/CentralNotice.i18n.php
——@@ -18,6 +18,7 @@
1919 'centralnotice-end-date' => 'End date',
2020 'centralnotice-enabled' => 'Enabled',
2121 'centralnotice-modify' => 'Submit',
 22+ 'centralnotice-save-banner' => 'Save banner',
2223 'centralnotice-preview' => 'Preview',
2324 'centralnotice-add-new' => 'Add a new campaign',
2425 'centralnotice-remove' => 'Remove',
——@@ -57,6 +58,7 @@
5859 'centralnotice-hours' => 'Hour',
5960 'centralnotice-min' => 'Minute',
6061 'centralnotice-project-lang' => 'Project language',
 62+ 'centralnotice-select' => 'Select',
6163 'centralnotice-project-name' => 'Project name',
6264 'centralnotice-start-date' => 'Start date',
6365 'centralnotice-start-time' => 'Start time (UTC)',
——@@ -94,7 +96,15 @@
9597 'centralnotice-clone-notice' => 'Create a copy of the banner',
9698 'centralnotice-clone-name' => 'Name',
9799 'centralnotice-preview-all-template-translations' => 'Preview all available translations of banner',
98 -
 100+ 'centralnotice-insert' => 'Insert',
 101+ 'centralnotice-hide-button' => 'Hide Button',
 102+ 'centralnotice-collapse-button' => 'Collapse Button',
 103+ 'centralnotice-expand-button' => 'Expand Button',
 104+ 'centralnotice-translate-button' => 'Help Translate Button',
 105+ 'centralnotice-donate-button' => 'Donate Button',
 106+ 'centralnotice-expanded-banner' => 'Expanded banner',
 107+ 'centralnotice-collapsed-banner' => 'Collapsed banner',
 108+
99109 'right-centralnotice-admin' => 'Manage central notices',
100110 'right-centralnotice-translate' => 'Translate central notices',
101111
——@@ -121,6 +131,7 @@
122132 'centralnotice-end-date' => '{{Identical|End date}}',
123133 'centralnotice-enabled' => '{{Identical|Enabled}}',
124134 'centralnotice-modify' => '{{Identical|Submit}}',
 135+ 'centralnotice-save-banner' => 'Label for the submit button which saves a CentralNotice banner.',
125136 'centralnotice-preview' => '{{Identical|Preview}}',
126137 'centralnotice-remove' => '{{Identical|Remove}}',
127138 'centralnotice-translate-heading' => 'Fieldset label. $1 is a name of a template.',
Index: trunk/extensions/CentralNotice/SpecialNoticeTemplate.php
——@@ -189,21 +189,23 @@
190190 $htmlOut .= Xml::hidden( 'wpMethod', 'addTemplate' );
191191 $htmlOut .= Xml::tags( 'p', null,
192192 Xml::inputLabel(
193 - wfMsg( 'centralnotice-template-name' ),
 193+ wfMsg( 'centralnotice-template-name' ) . ":",
194194 'templateName',
195195 'templateName',
196196 25
197197 )
198198 );
 199+ $htmlOut .= Xml::fieldset( wfMsg( 'centralnotice-template' ) );
199200 $htmlOut .= Xml::tags( 'p', null,
200201 Xml::textarea( 'templateBody', '', 60, 20 )
201202 );
 203+ $htmlOut .= Xml::closeElement( 'fieldset' );
202204 $htmlOut .= Xml::hidden( 'authtoken', $wgUser->editToken() );
203205
204206 // Submit button
205207 $htmlOut .= Xml::tags( 'div',
206208 array( 'class' => 'cn-buttons' ),
207 - Xml::submitButton( wfMsg( 'centralnotice-modify' ) )
 209+ Xml::submitButton( wfMsg( 'centralnotice-save-banner' ) )
208210 );
209211
210212 $htmlOut .= Xml::closeElement( 'form' );

Follow-up revisions

RevisionCommit summaryAuthorDate
r70541Follow-up r70529: Consistent casingraymond06:04, 6 August 2010
r70644moving all colons to message file per comments at r70529 and r70531kaldari19:41, 7 August 2010
r70778localization improvements per comments at r70529 and r70531kaldari20:25, 9 August 2010

Comments

#Comment by Siebrand (talk | contribs)   12:11, 7 August 2010

Contains hard coded colons and references to messages that should be added using "{{int:key}}"

#Comment by Kaldari (talk | contribs)   20:33, 9 August 2010

fixed in r70778

#Comment by Kaldari (talk | contribs)   18:45, 7 August 2010

I'm confused. If the colons aren't supposed to be hard-coded in the page code and they aren't supposed to be hard-coded in the messages file, how do you generate them? Or are we just not supposed to use colons at all?

I don't understand what you mean by messages that should be added using "⧼key⧽". Which messages are those? Is this fixed by r70611 and r70612? I didn't realize that "centralnotice-template-" was reserved when I wrote this revision, but I fixed that in the aforementioned revisions.

#Comment by MaxSem (talk | contribs)   18:52, 7 August 2010

See Localisation#Avoid patchwork messages. Instead of gluing several messages together, you should create message "Select: $1" and provide the link as a parameter to wfMsg().

#Comment by Kaldari (talk | contribs)   19:09, 7 August 2010

Ah, thanks for the pointer! I think that makes sense for the "Select:" interface. For the Insert and button messages, however, the display of the messages will eventually be dynamic depending on which type of banner they are building, see http://meta.wikimedia.org/wiki/CentralNotice_upgrades. So those need to be discrete messages.

Also, I see now that colons should always be in the messages file. I'll get that fixed as well.

#Comment by Siebrand (talk | contribs)   19:30, 7 August 2010

Sorry, I could have been clearer with regards to the "int:" thing.

If you add a button with set text and have a checkbox for that, it's best to re-use the message with int:. Something like: 

'centralnotice-collapse-button' => 'Add "{{int:message-key}}" button',

Doing it this way will create the best localised experience.

#Comment by Kaldari (talk | contribs)   19:45, 7 August 2010

Ah, I understand! Thanks.

#Comment by Kaldari (talk | contribs)   19:39, 7 August 2010

Actually, could you give me a bit of advice on the best way to implement this using "{{int:key}}". I need an interface like the following:

Select: <a href=...>All</a>, <a href=...>None</a>, <a href=...>Top 10 languages</a>

Is the best way to do this:

'Select: $1All$4, $2None$4, $3Top 10 languages$4'?

#Comment by Kaldari (talk | contribs)   19:47, 7 August 2010

Nevermind, I think I have it figured out now.

#Comment by Tim Starling (talk | contribs)   09:51, 20 August 2010

Appears to be all resolved now.

Status & tagging log