r99837 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r99836‎ | r99837 | r99838 >
Date:01:49, 15 October 2011
Author:jeroendedauw
Status:reverted (Comments)
Tags:
Comment:
use jquery fqncybox for rules display
Modified paths:
  • /trunk/extensions/Contest/Contest.i18n.php (modified) (history)
  • /trunk/extensions/Contest/Contest.php (modified) (history)
  • /trunk/extensions/Contest/resources/contest.special.welcome.js (modified) (history)
  • /trunk/extensions/Contest/specials/SpecialContestWelcome.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Contest/Contest.i18n.php
@@ -108,7 +108,8 @@
109109
110110 // Special:ContestWelcome
111111 'contest-welcome-unknown' => 'There is no contest with the provided name.',
112 - 'contest-welcome-rules' => 'In order to participate, you must agree to [[$1|the contest rules]].',
 112+ 'contest-welcome-rules' => 'In order to participate, you must agree to', // js i18n
 113+ 'contest-welcome-rules-link' => 'the contest rules', // js i18n
113114 'contest-welcome-signup' => 'Signup now',
114115 'contest-welcome-js-off' => 'Contest uses JavaScript for an improved interface. Your browser either does not support JavaScript or has JavaScript turned off.',
115116 'contest-welcome-accept-challenge' => 'Challenge Accepted',
Index: trunk/extensions/Contest/specials/SpecialContestWelcome.php
@@ -46,7 +46,9 @@
4747 $out->returnToMain();
4848 }
4949 else if ( $contest->getField( 'status' ) !== Contest::STATUS_ACTIVE ) {
50 - // TODO: show message
 50+ $this->showWarning( 'contest-signup-finished' );
 51+ $out->addHTML( '<br /><br /><br /><br />' );
 52+ $out->returnToMain();
5153 }
5254 else {
5355 $this->showEnabledPage( $contest );
@@ -158,8 +160,13 @@
159161 * @param Contest $contest
160162 */
161163 protected function showRules( Contest $contest ) {
162 - // TODO: we might want to have a pop-up with the content here, instead of a link to the page.
163 - $this->getOutput()->addWikiMsgArray( 'contest-welcome-rules', $contest->getField( 'rules_page' ) );
 164+ $this->getOutput()->addHTML( Html::element(
 165+ 'div',
 166+ array(
 167+ 'id' => 'contest-rules',
 168+ 'data-rules' => ContestUtils::getParsedArticleContent( $contest->getField( 'rules_page' ) )
 169+ )
 170+ ) );
164171 }
165172
166173 /**
Index: trunk/extensions/Contest/Contest.php
@@ -210,7 +210,9 @@
211211 'jquery.contestChallenges', 'jquery.fancybox',
212212 ),
213213 'messages' => array(
214 - 'contest-welcome-select-header'
 214+ 'contest-welcome-select-header',
 215+ 'contest-welcome-rules',
 216+ 'contest-welcome-rules-link',
215217 )
216218 );
217219
Index: trunk/extensions/Contest/resources/contest.special.welcome.js
@@ -15,6 +15,27 @@
1616 mw.config.get( 'ContestConfig' )
1717 );
1818
 19+ $rules = $( '#contest-rules' );
 20+
 21+ $div = $( '<div />' ).attr( {
 22+ 'style': 'display:none'
 23+ } ).html( $( '<div />' ).attr( { 'id': 'contest-rules-div' } ).html( $rules.attr( 'data-rules' ) ) );
 24+
 25+ // TODO: fix very ugly message construction.
 26+ $a = $( '<a />' ).text( mw.msg( 'contest-welcome-rules-link' ) ).attr( { 'href': '#contest-rules-div' } );
 27+ $p = $( '<p />' ).text( mw.msg( 'contest-welcome-rules' ) + ' ' ).append( $a ).append( '.' );
 28+
 29+ $rules.html( $p ).append( $div );
 30+
 31+ $a.fancybox( {
 32+ 'width' : '75%',
 33+// 'height' : '75%',
 34+// 'autoScale' : true,
 35+ 'transitionIn' : 'none',
 36+ 'transitionOut' : 'none',
 37+ 'type' : 'inline',
 38+// 'autoDimensions': false
 39+ } );
1940 } );
2041
2142 })( window.jQuery, window.mediaWiki );
\ No newline at end of file

Follow-up revisions

RevisionCommit summaryAuthorDate
r100433* (bug 31860, bug 31861) Revert most of r99837: i18n construction regression,...brion18:07, 21 October 2011
r100434MFT r100433: fix bug 31860, 31861 in Contest extension by reverting most of r...brion18:09, 21 October 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   18:02, 21 October 2011

Bad i18n construction; also this popup box is annoying and has accessibility problems (bug 31860, bug 31861). Recommend revert.

#Comment by Brion VIBBER (talk | contribs)   18:22, 21 October 2011

Did the revert for now (r100433). Things to test/fix if considering adding this back:

  1. embed the link into the first message via a $ parameter (and replace it instead of appending), for proper i18n construction. not all languages will put that part of the sentence at the end!
  2. set the link's href to point at the actual page, so you can right-click or middle-click to copy/paste the URL or open in new window. you can still handle a click(), just remember to do e.preventDefault() to cancel default handling
  3. see if it's possible to let the box expand to full height rather than embedded scrolling; on touchscreen systems eg iPad or iPhone) it doesn't always work
  4. add a "open in full window" link within the popup so you can get to the full page for bookmarking

Status & tagging log