r63355 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r63354‎ | r63355 | r63356 >
Date:02:36, 7 March 2010
Author:jeroendedauw
Status:resolved (Comments)
Tags:
Comment:
Added new fields to the storysubmission form and restructured the old ones somewhat.
Modified paths:
  • /trunk/extensions/Storyboard/Storyboard.i18n.php (modified) (history)
  • /trunk/extensions/Storyboard/tags/Storyboard/storyboard.css (modified) (history)
  • /trunk/extensions/Storyboard/tags/Storyboard/storyboard.js (modified) (history)
  • /trunk/extensions/Storyboard/tags/Storysubmission/Storysubmission_body.php (modified) (history)
  • /trunk/extensions/Storyboard/tags/Storysubmission/storysubmission.css (added) (history)
  • /trunk/extensions/Storyboard/tags/Storysubmission/storysubmission.js (added) (history)

Diff [purge]

Index: trunk/extensions/Storyboard/Storyboard.i18n.php
——@@ -29,12 +29,16 @@
3030
3131 // Story submission
3232 'storyboard-yourname' => 'Your name',
33 - 'storyboard-location' => 'Where do you live?',
 33+ 'storyboard-location' => 'Your location',
3434 'storyboard-occupation' => 'Your occupation',
35 - 'storyboard-story' => "What's your story",
 35+ 'storyboard-story' => "Your story",
3636 'storyboard-photo' => 'Have a photo of yourself? Why not share it?',
37 - 'storyboard-contact' => 'E-mail address or telephone number',
 37+ 'storyboard-contact' => 'Your E-mail address or telephone number',
3838 'storyboard-agreement' => 'I agree with the publication of this story.',
 39+ 'storyboard-charsleft' => '($1 {{PLURAL:$1|character|characters}} left)',
 40+ 'storyboard-cannotbelonger' => 'Your story may not exceed $ characters!',
 41+ 'storyboard-charsneeded' => '($1 more {{PLURAL:$1|character|characters}} needed)',
 42+ 'storyboard-needtoagree' => 'You need to agree to the publication of your story to submit it.'
3943 );
4044
4145 /** Afrikaans (Afrikaans)
Index: trunk/extensions/Storyboard/tags/Storyboard/storyboard.css
——@@ -1,11 +1,11 @@
22 /**
3 - * Storyboard css.
 3+ * Css for <storyboard> tags of the Storyboard extension.
44 *
55 * @file Storyboard.css
66 * @ingroup Storyboard
77 *
88 * @author Jeroen De Dauw
9 - */
 9+ */
1010
1111 .storyboard {
1212 border: 1px solid #ddd;
Index: trunk/extensions/Storyboard/tags/Storyboard/storyboard.js
——@@ -1,5 +1,8 @@
22 /**
3 - * JavaScript added for <storyboard> tags
 3+ * JavaScript for <storyboard> tags.
 4+ *
 5+ * @author Jeroen De Dauw
 6+ * @ingroup Storyboard
47 */
58
69 (function($) {
——@@ -72,7 +75,7 @@
7376 ) //TODO
7477 .appendTo( $header );
7578
76 - $storyBody.append( $( "<div />" ).text( '.' ).addClass( "storyboard-text" ) ); // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris dapibus, nibh vitae blandit tincidunt, risus libero vehicula mauris, nec interdum mi lectus lobortis turpis. Suspendisse vel magna dapibus purus iaculis hendrerit nec ut lectus. Nullam in turpis sed elit volutpat accumsan id quis lectus. Phasellus a felis lectus. Donec erat neque, tincidunt sit amet tempus non, malesuada vel justo. Vestibulum eget magna enim, quis malesuada lorem. Integer rutrum scelerisque adipiscing. Proin feugiat tincidunt ultrices. Vivamus at justo turpis, ut porta mi. Fusce nisl eros, luctus non accumsan eget, varius id urna. Quisque hendrerit, neque eu varius sollicitudin, lacus nisl auctor odio, eget egestas turpis sapien ut diam. Quisque ultricies consequat erat in tempor. Nam ut libero ac massa volutpat vestibulum vel sed leo. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Aenean pharetra, nisi ut dignissim semper, nunc lectus elementum dolor, sit amet blandit mauris diam ut nisi. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Etiam fringilla ultricies dapibus. Donec tristique congue lorem eget varius. Nunc nunc orci, molestie et elementum eget, pulvinar ac ante. Nam fermentum est ut lorem luctus suscipit consectetur ligula gravida. Phasellus non magna sit amet lectus feugiat malesuada eu non lorem. Nam commodo fermentum mauris, sed vehicula risus molestie et. Sed nisl neque, mollis sit amet malesuada ac, bibendum vitae mauris. Quisque dictum viverra eros quis gravida. Etiam vitae augue risus. Donec at orci vitae mauris luctus porttitor.
 79+ $storyBody.append( $( "<div />" ).text( story.text ).addClass( "storyboard-text" ) ); // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris dapibus, nibh vitae blandit tincidunt, risus libero vehicula mauris, nec interdum mi lectus lobortis turpis. Suspendisse vel magna dapibus purus iaculis hendrerit nec ut lectus. Nullam in turpis sed elit volutpat accumsan id quis lectus. Phasellus a felis lectus. Donec erat neque, tincidunt sit amet tempus non, malesuada vel justo. Vestibulum eget magna enim, quis malesuada lorem. Integer rutrum scelerisque adipiscing. Proin feugiat tincidunt ultrices. Vivamus at justo turpis, ut porta mi. Fusce nisl eros, luctus non accumsan eget, varius id urna. Quisque hendrerit, neque eu varius sollicitudin, lacus nisl auctor odio, eget egestas turpis sapien ut diam. Quisque ultricies consequat erat in tempor. Nam ut libero ac massa volutpat vestibulum vel sed leo. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Aenean pharetra, nisi ut dignissim semper, nunc lectus elementum dolor, sit amet blandit mauris diam ut nisi. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Etiam fringilla ultricies dapibus. Donec tristique congue lorem eget varius. Nunc nunc orci, molestie et elementum eget, pulvinar ac ante. Nam fermentum est ut lorem luctus suscipit consectetur ligula gravida. Phasellus non magna sit amet lectus feugiat malesuada eu non lorem. Nam commodo fermentum mauris, sed vehicula risus molestie et. Sed nisl neque, mollis sit amet malesuada ac, bibendum vitae mauris. Quisque dictum viverra eros quis gravida. Etiam vitae augue risus. Donec at orci vitae mauris luctus porttitor.
7780 $div.append( $storyBody );
7881 }
7982 $storyboard.html( $div );
Index: trunk/extensions/Storyboard/tags/Storysubmission/Storysubmission_body.php
——@@ -7,6 +7,9 @@
88 * @ingroup Storyboard
99 *
1010 * @author Jeroen De Dauw
 11+ *
 12+ * Notice: This class is designed with the idea that only one storysubmission form is placed
 13+ * on a single page, and might not work properly when multiple are placed on a page.
1114 */
1215
1316 if ( !defined( 'MEDIAWIKI' ) ) {
——@@ -34,64 +37,83 @@
3538 }
3639
3740 private static function getFrom( $parser, $args ) {
38 - global $egStorysubmissionWidth;
 41+ global $wgOut, $egStoryboardScriptPath, $egStorysubmissionWidth, $egStoryboardMaxStoryLen, $egStoryboardMinStoryLen;
3942
 43+ $wgOut->addStyle( $egStoryboardScriptPath . '/tags/Storysubmission/storysubmission.css' );
 44+ $wgOut->addScriptFile( $egStoryboardScriptPath . '/tags/Storysubmission/storysubmission.js' );
 45+
4046 $fieldSize = 50;
4147
4248 $width = StoryboardUtils::getDimension( $args, 'width', $egStorysubmissionWidth );
 49+ $maxLen = array_key_exists('maxlength', $args) && is_numeric($args['maxlength']) ? $args['maxlength'] : $egStoryboardMaxStoryLen;
 50+ $minLen = array_key_exists('minlength', $args) && is_numeric($args['minlength']) ? $args['minlength'] : $egStoryboardMinStoryLen;
4351
44 - $url = $parser->getTitle()->getLocalURL( 'action=submit' );
 52+ $submissionUrl = $parser->getTitle()->getLocalURL( 'action=submit' ); // TODO: fix parameters
4553
4654 $formBody = "<table width='$width'>";
4755
4856 $formBody .= '<tr>' .
4957 Html::element( 'td', array('width' => '100%'), wfMsg( 'storyboard-yourname' ) ) .
50 - '<td>' . Html::element(
51 - 'input',
52 - array( 'id' => 'name', 'name' => 'name', 'type' => 'text', 'size' => $fieldSize ),
53 - null
 58+ '<td>' .
 59+ Html::input('name' ,'', 'text', array( 'size' => $fieldSize )
5460 ) . '</td></tr>';
5561
5662 $formBody .= '<tr>' .
5763 Html::element( 'td', array('width' => '100%'), wfMsg( 'storyboard-location' ) ) .
58 - '<td>' . Html::element(
59 - 'input',
60 - array(' id' => 'location', 'name' => 'location', 'type' => 'text', 'size' => $fieldSize ),
61 - null
 64+ '<td>' . Html::input('location' ,'', 'text', array( 'size' => $fieldSize )
6265 ) . '</td></tr>';
6366
6467 $formBody .= '<tr>' .
6568 Html::element( 'td', array('width' => '100%'), wfMsg( 'storyboard-occupation' ) ) .
66 - '<td>' . Html::element(
67 - 'input',
68 - array( 'id' => 'occupation', 'name' => 'occupation', 'type' => 'text', 'size' => $fieldSize ),
69 - null
 69+ '<td>' . Html::input('occupation' ,'', 'text', array( 'size' => $fieldSize )
7070 ) . '</td></tr>';
7171
7272 $formBody .= '<tr>' .
7373 Html::element( 'td', array('width' => '100%'), wfMsg( 'storyboard-contact' ) ) .
74 - '<td>' . Html::element(
75 - 'input',
76 - array( 'id' => 'contact', 'name' => 'contact', 'type' => 'text', 'size' => $fieldSize ),
77 - null
 74+ '<td>' . Html::input('contact' ,'', 'text', array( 'size' => $fieldSize )
7875 ) . '</td></tr>';
79 -
80 - $formBody .= '<tr><td colspan="2">' . // TODO: add 'x-chars left' feature
81 - wfMsg( 'storyboard-story' ) . '<br />' .
 76+
 77+ $formBody .= '<tr><td colspan="2">' .
 78+ wfMsg( 'storyboard-story' ) .
8279 Html::element(
 80+ 'div',
 81+ array('class' => 'storysubmission-charcount', 'id' => 'storysubmission-charlimitinfo'),
 82+ wfMsgExt( 'storyboard-charsneeded', 'parsemag', $minLen )
 83+ ) .
 84+ '<br />' .
 85+ Html::element(
8386 'textarea',
84 - array( 'id' => 'story', 'rows' => 7 ),
 87+ array(
 88+ 'id' => 'story',
 89+ 'rows' => 7,
 90+ 'onkeyup' => "stbValidateStory( this, $minLen, $maxLen, 'storysubmission-charlimitinfo', 'storysubmission-button' )",
 91+ // TODO: make disabled when JS is enabled
 92+ ),
8593 null
8694 ) .
8795 '</td></tr>';
 96+
 97+ // TODO: add upload functionality
 98+
 99+ $formBody .= '<tr><td colspan="2"><input type="checkbox" id="storyboard-agreement" />&nbsp;' .
 100+ htmlspecialchars( wfMsg( 'storyboard-agreement' ) ) .
 101+ '</td></tr>';
88102
89 - $formBody .= '<tr><td colspan="2">' . Html::input( '', wfMsg( 'htmlform-submit' ), 'submit' ) . '</td></tr>';
90 -
 103+ $formBody .= '<tr><td colspan="2">' .
 104+ Html::input( '', wfMsg( 'htmlform-submit' ), 'submit', array('id' => 'storysubmission-button') ) .
 105+ '</td></tr>';
 106+
91107 $formBody .= '</table>';
92108
93109 return Html::rawElement(
94110 'form',
95 - array( 'id' => 'storyform', 'name' => 'storyform', 'method' => 'post', 'action' => $url ),
 111+ array(
 112+ 'id' => 'storyform',
 113+ 'name' => 'storyform',
 114+ 'method' => 'post',
 115+ 'action' => $submissionUrl,
 116+ 'onsubmit' => 'return stbValidateSubmission( "storyboard-agreement" );'
 117+ ),
96118 $formBody
97119 );
98120 }
Index: trunk/extensions/Storyboard/tags/Storysubmission/storysubmission.css
——@@ -0,0 +1,13 @@
 2+/**
 3+ * Css for <storysubmission> tags of the Storyboard extension.
 4+ *
 5+ * @file Storysubmission.css
 6+ * @ingroup Storyboard
 7+ *
 8+ * @author Jeroen De Dauw
 9+ */
 10+
 11+.storysubmission-charcount {
 12+ display: inline;
 13+ float: right;
 14+}
\ No newline at end of file
Property changes on: trunk/extensions/Storyboard/tags/Storysubmission/storysubmission.css
___________________________________________________________________
Name: svn:eol-style
115 + native
Index: trunk/extensions/Storyboard/tags/Storysubmission/storysubmission.js
——@@ -0,0 +1,37 @@
 2+/**
 3+ * JavaScript for <storysubmission> tags.
 4+ *
 5+ * @author Jeroen De Dauw
 6+ * @ingroup Storyboard
 7+ */
 8+
 9+function stbValidateStory( textarea, lowerLimit, upperLimit, infodiv, submissionButton ) {
 10+ var button = document.getElementById( submissionButton );
 11+ button.disabled = !stbLimitChars( textarea, lowerLimit, upperLimit, infodiv );
 12+}
 13+
 14+function stbLimitChars(textarea, lowerLimit, upperLimit, infodiv) {
 15+ var text = textarea.value;
 16+ var textlength = text.length;
 17+ var info = document.getElementById( infodiv );
 18+
 19+ if(textlength > upperLimit) {
 20+ info.innerHTML = 'Your story may not exceed ' + upperLimit + ' characters!'; // TODO: i18n
 21+ textarea.value = text.substr( 0, upperLimit );
 22+ return true;
 23+ } else if (textlength < lowerLimit) {
 24+ info.innerHTML = '('+ ( lowerLimit - textlength ) + ' more characters needed)'; // TODO: i18n
 25+ return false;
 26+ } else {
 27+ info.innerHTML = '(' + ( upperLimit - textlength ) + ' characters left)'; // TODO: i18n
 28+ return true;
 29+ }
 30+}
 31+
 32+function stbValidateSubmission( termsCheckbox ) {
 33+ var agreementValid = document.getElementById( termsCheckbox ).checked;
 34+ if (!agreementValid) {
 35+ alert( 'You need to agree to the publication of your story to submit it.' ); // TODO: i18n
 36+ }
 37+ return agreementValid;
 38+}
\ No newline at end of file
Property changes on: trunk/extensions/Storyboard/tags/Storysubmission/storysubmission.js
___________________________________________________________________
Name: svn:eol-style
139 + native

Follow-up revisions

RevisionCommit summaryAuthorDate
r63362Partial follow up to r63355jeroendedauw16:53, 7 March 2010

Comments

#Comment by Catrope (talk | contribs)   11:09, 7 March 2010 
+	'storyboard-cannotbelonger' => 'Your story may not exceed $ characters!',

You have a typo here ($ vs. $1). 

+					'onkeyup' => "stbValidateStory( this, $minLen, $maxLen, 'storysubmission-charlimitinfo', 'storysubmission-button' )",
+					// TODO: make disabled when JS is enabled

You should use $( '#story' ).keyup( function() { ... } ); instead of using the onkeyup attribute. 

+				'onsubmit' => 'return stbValidateSubmission( "storyboard-agreement" );',

Same here. 

+		info.innerHTML = 'Your story may not exceed ' + upperLimit + ' characters!'; // TODO: i18n
+		textarea.value = text.substr( 0, upperLimit );

This sounds evil; why not do it the way identi.ca/Twitter handle this (show a big red negative number but allow the user to continue typing)?

#Comment by Jeroen De Dauw (talk | contribs)   18:48, 7 March 2010

Why should I go use the jQuery approach rather then regular JS here? The regular JS works, and there are no issues with it (so why fix what's not broken?). Also, doing this with jQuery would make jQuery required on this page, which is it not already.

Other remarks have been fixed in r63362

#Comment by Catrope (talk | contribs)   20:02, 7 March 2010

Fair enough.

Status & tagging log