r62977 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r62976‎ | r62977 | r62978 >
Date:22:15, 25 February 2010
Author:jeroendedauw
Status:ok (Comments)
Tags:
Comment:
Updated js, css & images and added non finished API module for the storyboard tag
Modified paths:
  • /trunk/extensions/Storyboard/Storyboard.i18n.php (modified) (history)
  • /trunk/extensions/Storyboard/Storyboard.php (modified) (history)
  • /trunk/extensions/Storyboard/Storyboard_Settings.php (modified) (history)
  • /trunk/extensions/Storyboard/api (added) (history)
  • /trunk/extensions/Storyboard/api/ApiStoryboardStoriesFeed.php (added) (history)
  • /trunk/extensions/Storyboard/images/storyboard-background.png (deleted) (history)
  • /trunk/extensions/Storyboard/images/storyboard-loader.gif (modified) (history)
  • /trunk/extensions/Storyboard/images/storyboard-result-spacer.png (deleted) (history)
  • /trunk/extensions/Storyboard/tags/Storyboard/Stories.php (deleted) (history)
  • /trunk/extensions/Storyboard/tags/Storyboard/Storyboard_body.php (modified) (history)
  • /trunk/extensions/Storyboard/tags/Storyboard/jquery.ajaxscroll.min.js (added) (history)
  • /trunk/extensions/Storyboard/tags/Storyboard/storyboard.css (modified) (history)
  • /trunk/extensions/Storyboard/tags/Storyboard/storyboard.js (deleted) (history)
  • /trunk/extensions/Storyboard/tags/Storysubmission/Storysubmission_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Storyboard/images/storyboard-result-spacer.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Index: trunk/extensions/Storyboard/images/storyboard-background.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Index: trunk/extensions/Storyboard/images/storyboard-loader.gif
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: trunk/extensions/Storyboard/api/ApiStoryboardStoriesFeed.php
@@ -0,0 +1,111 @@
 2+<?php
 3+/**
 4+ * API extension for Storyboard.
 5+ *
 6+ * @file ApiStories.php
 7+ * @ingroup Storyboard
 8+ *
 9+ * @author Jeroen De Dauw
 10+ *
 11+ * This program is free software; you can redistribute it and/or modify
 12+ * it under the terms of the GNU General Public License as published by
 13+ * the Free Software Foundation; either version 2 of the License, or
 14+ * (at your option) any later version.
 15+ *
 16+ * This program is distributed in the hope that it will be useful,
 17+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
 18+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 19+ * GNU General Public License for more details.
 20+ *
 21+ * You should have received a copy of the GNU General Public License along
 22+ * with this program; if not, write to the Free Software Foundation, Inc.,
 23+ * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 24+ * http://www.gnu.org/copyleft/gpl.html
 25+ */
 26+
 27+if ( !defined( 'MEDIAWIKI' ) ) {
 28+ // Eclipse helper - will be ignored in production
 29+ require_once ( "ApiBase.php" );
 30+}
 31+
 32+/**
 33+ * This action returns the html for Stories to be displayed in a storyboard.
 34+ *
 35+ * @ingroup Storyboard
 36+ */
 37+class ApiStoryboardStoriesFeed extends ApiQueryBase {
 38+ public function __construct( $main, $action ) {
 39+ parent :: __construct( $main, $action );
 40+ }
 41+
 42+ /**
 43+ * Retrieve the stories from the database.
 44+ */
 45+ public function execute() {
 46+ // Get the requests parameters.
 47+ $params = $this->extractRequestParams();
 48+
 49+ // Get a slave db object to do read operations against.
 50+ $dbr = wfGetDB( DB_SLAVE );
 51+ }
 52+
 53+ /**
 54+ * TODO
 55+ * @see includes/api/ApiBase#getAllowedParams()
 56+ */
 57+ public function getAllowedParams() {
 58+ return array (
 59+ 'offset' => array (
 60+ ApiBase :: PARAM_DFLT => 0,
 61+ ApiBase :: PARAM_TYPE => 'integer',
 62+ ApiBase :: PARAM_MIN => 0,
 63+ ),
 64+ 'size' => array (
 65+ ApiBase :: PARAM_DFLT => 5,
 66+ ApiBase :: PARAM_TYPE => 'integer',
 67+ ApiBase :: PARAM_MIN => 1,
 68+ ApiBase :: PARAM_MAX => 100,
 69+ ),
 70+ );
 71+ }
 72+
 73+ /**
 74+ * (non-PHPdoc)
 75+ * @see includes/api/ApiBase#getParamDescription()
 76+ */
 77+ public function getParamDescription() {
 78+ return array (
 79+ 'offset' => 'Number of the first story to return',
 80+ 'size' => 'Amount of stories to return',
 81+ );
 82+ }
 83+
 84+ /**
 85+ * (non-PHPdoc)
 86+ * @see includes/api/ApiBase#getDescription()
 87+ */
 88+ public function getDescription() {
 89+ return 'This module returns stories for a storyboard';
 90+ }
 91+
 92+ /**
 93+ * (non-PHPdoc)
 94+ * @see includes/api/ApiBase#getExamples()
 95+ */
 96+ protected function getExamples() {
 97+ return array (
 98+ 'api.php?action=stories',
 99+ 'api.php?action=stories&offset=42',
 100+ 'api.php?action=stories&offset=4&size=2',
 101+ );
 102+ }
 103+
 104+ /**
 105+ * TODO
 106+ * @see includes/api/ApiBase#getVersion()
 107+ */
 108+ public function getVersion() {
 109+ return __CLASS__ . ': ';
 110+ }
 111+
 112+}
\ No newline at end of file
Property changes on: trunk/extensions/Storyboard/api/ApiStoryboardStoriesFeed.php
___________________________________________________________________
Added: svn:eol-style
1113 + native
Index: trunk/extensions/Storyboard/Storyboard.i18n.php
@@ -23,6 +23,7 @@
2424 // Story review
2525 'storyboard-storyreview' => 'Story review',
2626 'storyboard-publish' => 'Publish',
 27+ 'storyboard-unpublish' => 'Unpublish',
2728 'storyboard-reviewed' => 'Reviewed',
2829 'storyboard-unreviewed' => 'Unreviewed',
2930 );
Index: trunk/extensions/Storyboard/Storyboard.php
@@ -46,6 +46,10 @@
4747 $wgSpecialPages['StoryReview'] = 'SpecialStoryReview';
4848 $wgSpecialPageGroups['StoryReview'] = 'contribution';
4949
 50+// API
 51+$wgAutoloadClasses['ApiStoryboardStoriesFeed'] = "{$egStoryboardDir}api/ApiStoryboardStoriesFeed.php";
 52+$wgAPIListModules['stories'] = 'ApiStoryboardStoriesFeed';
 53+
5054 // Hooks
5155 $wgHooks['ParserFirstCallInit'][] = 'efStoryboardParserFirstCallInit';
5256 $wgHooks['LoadExtensionSchemaUpdates'][] = 'efStoryboardSchemaUpdate';
@@ -80,7 +84,7 @@
8185 }
8286
8387 function efStoryboardParserFirstCallInit( &$parser ) {
84 - $parser->setHook( 'storyboard', array('TagStoryboard', 'render') );
85 - $parser->setHook( 'storysubmission', array('TagStorysubmission', 'render') );
 88+ $parser->setHook( 'storyboard', array( 'TagStoryboard', 'render' ) );
 89+ $parser->setHook( 'storysubmission', array( 'TagStorysubmission', 'render' ) );
8690 return true;
8791 }
Index: trunk/extensions/Storyboard/Storyboard_Settings.php
@@ -16,3 +16,12 @@
1717 if ( !defined( 'MEDIAWIKI' ) ) {
1818 die( 'Not an entry point.' );
1919 }
 20+
 21+$egStoryboardMaxStoryLen = 1000;
 22+$egStoryboardMinStoryLen = 10;
 23+
 24+$egStoryboardWidth = '80%';
 25+$egStoryboardHieght = 400;
 26+
 27+$egStoryboardBatchSize = 5;
 28+$egStoryboardBatchAmount = 2;
\ No newline at end of file
Index: trunk/extensions/Storyboard/tags/Storyboard/storyboard.js
@@ -1,87 +0,0 @@
2 - /**
3 - * Storyboard object.
4 - *
5 - * @file Storyboard.js
6 - * @ingroup Storyboard
7 - *
8 - * @author Jeroen De Dauw
9 - * @author Tha Devil (helpmij.nl)
10 - */
11 -
12 -/**
13 - * Constructor for the Storyboard object.
14 - */
15 -function Storyboard() {
16 - this.stories = [];
17 -
18 - this.pageNumber = 0;
19 - this.pageAmount = -1;
20 -
21 - this.isLoading = false;
22 -}
23 -
24 -/**
25 - * Scroll method.
26 - * Checks if new stories should be loaded, and calls loadAjax when this is the case.
27 - */
28 -Storyboard.prototype.scroll = function(event) {
29 - var board = event.data[0];
30 - var documentHeight = jQuery(document).height();
31 - var top = jQuery("#storyboard").scrollTop();
32 -
33 - var resultsHeight = 0;
34 - jQuery("#storyboard .storyboard-result").each(function() {
35 - resultsHeight += $(this).height();
36 - });
37 -
38 - var threshold = ( ( top / 2 ) + ( 0.01 * Math.pow(documentHeight, 2) ) ) / resultsHeight;
39 - if (threshold > 0.6) threshold = 0.6;
40 -
41 - if( top / resultsHeight >= threshold && !board.isLoading && board.pageNumber < board.pageAmount )
42 - {
43 - board.loadAjax();
44 - }
45 -};
46 -
47 -/**
48 - * LoadAjax method.
49 - * Requests and loads new stories into the storyboard.
50 - */
51 -Storyboard.prototype.loadAjax = function() {
52 - alert('load');
53 - var t = this;
54 -
55 - this.isLoading = true;
56 -
57 - jQuery("#storyboard").append(jQuery("<div />").attr("id","storyboard-loading"));
58 -
59 - jQuery.getJSON(storyboardPath + '/tags/Storyboard/Stories.php?&number=' + t.currentPage + '', function(json) {
60 -
61 - t.pageAmount = json.pageAmount;
62 - t.stories.push(json);
63 - t.isLoading = false;
64 -
65 - jQuery("#storyboard-loading").remove();
66 -
67 - t.display();
68 - });
69 -};
70 -
71 -/**
72 - * Display method.
73 - * Creates the storyboard layout.
74 - */
75 -Storyboard.prototype.display = function() {
76 - alert('display');
77 -};
78 -
79 -/**
80 - * Reload method.
81 - * Removes all stories from the storyboard and loads a new batch of stories.
82 - */
83 -Storyboard.prototype.reload = function() {
84 - jQuery("#storyboard").html("");
85 - this.stories = [];
86 - this.pageNumber = 0;
87 - this.loadAjax();
88 -};
\ No newline at end of file
Index: trunk/extensions/Storyboard/tags/Storyboard/Stories.php
@@ -1,11 +0,0 @@
2 -<?php
3 -
4 -/**
5 - * Webservice for fetching stories from the Storyboard extension.
6 - *
7 - * @file Stories.php
8 - * @ingroup Storyboard
9 - *
10 - * @author Jeroen De Dauw
11 - */
12 -
Index: trunk/extensions/Storyboard/tags/Storyboard/storyboard.css
@@ -1,93 +1,33 @@
22 /**
3 - * Storyboard tag css.
 3+ * Storyboard css.
44 *
55 * @file Storyboard.css
66 * @ingroup Storyboard
77 *
88 * @author Jeroen De Dauw
9 - * @author Tha Devil (helpmij.nl)
109 */
1110
12 -#storyboard
13 -{
14 - margin-left: 31px;
15 - width: 50%;
16 - height: 300px;
17 - overflow-y: scroll;
18 - overflow-x: hidden;
19 - font-family: Verdana;
20 - font-size: 10px;
 11+.ajaxscroll {
 12+border: 1px solid #ddd;
2113 }
22 -
23 -#storyboard-results
24 -{
25 - background: url(extensions/Storyboard/images/storyboard-background.png) repeat-y;
 14+.ajaxscroll .batch {
 15+ margin: 0;
 16+ padding: 0;
2617 }
27 -
28 -.storyboard-result
29 -{
30 - padding-bottom: 6px;
31 - background: url(extensions/Storyboard/images/storyboard-result-spacer.png) no-repeat bottom;
32 -}
33 -
34 -.storyboard-result .storyboard-header
35 -{
36 - margin: 10px 0px 10px 15px;
37 -}
38 -
39 -.storyboard-result .storyboard-header .storyboard-title
40 -{
41 - font: 16px "Lucida Sans", Verdana;
42 - font-weight: bold;
43 - width: 100px;
44 - height: 21px;
45 - overflow: hidden;
46 - float: left;
47 -}
48 -
49 -.storyboard-result
50 -{
51 - margin-left: 12px;
52 - margin-right: 15px;
53 -}
54 -
55 -.storyboard-result
56 -{
57 - float: left;
58 - width: 146px;
59 - height: 112px;
60 - margin-left: 3px;
61 - margin-right: 3px;
62 - margin-top: 10px;
63 - overflow: hidden;
 18+.ajaxscroll .box {
 19+ display: inline-block;
 20+ border: 1px solid #eee;
 21+ width: 99%;
 22+ height: 150px;
6423 text-align: center;
 24+ margin: 5px !important;
 25+ margin-bottom: 25px;
6526 }
66 -
67 -.storyboard-result
68 -{
69 - display: none;
 27+.ajaxscroll .empty .box {
 28+ border: 1px solid #ddd;
 29+ background: #FCFCFC url(../../images/storyboard-loader.gif) no-repeat scroll center center;
7030 }
71 -
72 -#storyboard-loading
73 -{
74 - background-image: url(extensions/Storyboard/images/storyboard-loader.gif);
75 - width:31px;
76 - height:31px;
77 - margin:0 auto;
78 - margin-top: 10px;
79 -}
80 -
81 -.storyboard-footer
82 -{
83 - height: 40px;
84 - padding-top: 20px;
85 - margin-left: 15px;
86 -}
87 -
88 -.storyboard-sharing
89 -{
90 - margin-left: 750px;
91 - height: 30px;
92 - width: 110px;
93 - float: left;
94 -}
 31+#storyboard {
 32+ height: 400px;
 33+ width: 80%;
 34+}
\ No newline at end of file
Index: trunk/extensions/Storyboard/tags/Storyboard/Storyboard_body.php
@@ -16,20 +16,19 @@
1717 class TagStoryboard {
1818
1919 public static function render( $input, $args, $parser, $frame ) {
20 - global $wgOut, $wgJsMimeType, $egStoryboardScriptPath;
 20+ global $wgOut, $wgJsMimeType, $egStoryboardScriptPath, $wgScriptPath;
2121
2222 $wgOut->addStyle($egStoryboardScriptPath . '/tags/Storyboard/storyboard.css');
2323 $wgOut->includeJQuery();
24 - $wgOut->addScriptFile($egStoryboardScriptPath . '/tags/Storyboard/storyboard.js');
 24+ $wgOut->addScriptFile($egStoryboardScriptPath . '/tags/Storyboard/jquery.ajaxscroll.min.js');
2525
26 - $output = <<<END
27 -<script type="$wgJsMimeType">var storyboardPath = '$egStoryboardScriptPath';</script>
28 -<div id="storyboard"></div>
 26+ $output = <<<EOT
 27+<div class="ajaxscroll" id="storyboard">
2928 <script type="$wgJsMimeType"> /*<![CDATA[*/
30 - var storyboard = new Storyboard();
31 - storyboard.loadAjax();
 29+jQuery(function(){ jQuery('#storyboard').ajaxScroll({ updateBatch: updateStoryboard, batchSize: 5, batchNum: 2 }); });
 30+function updateStoryboard(obj){ obj.load('$wgScriptPath/api.php?action=stories&offset=' + obj.attr('offset')); }
3231 /*]]>*/ </script>
33 -END;
 32+EOT;
3433
3534 return array($output, 'noparse' => 'true', 'isHTML' => 'true');
3635 }
Index: trunk/extensions/Storyboard/tags/Storyboard/jquery.ajaxscroll.min.js
@@ -0,0 +1,13 @@
 2+/**
 3+ * AjaxScroll v0.1 (jQuery Plugins)
 4+ *
 5+ * @author Timmy Tin (ycTIN)
 6+ * @license GPL
 7+ * @version 0.1
 8+ * @copyright Timmy Tin (ycTIN)
 9+ * @website http://project.yctin.com/ajaxscroll
 10+ *
 11+ */
 12+(function($) {
 13+ $.fn.ajaxScroll=function(opt){opt=jQuery.extend({batchNum:5,batchSize:30,horizontal:false,batchTemplate:null,boxTemplate:null,batchClass:"batch",boxClass:"box",emptyBatchClass:"empty",scrollPaneClass:"scrollpane",lBound:"auto",uBound:"auto",eBound:"auto",maxOffset:1000,scrollDelay:600,endDelay:100,updateBatch:null,updateEnd:null},opt);return this.each(function(){var ele=this;var $me=jQuery(this);var $sp;var fnEnd,fnScroll;var offset=0;var lsp=-1;_css();opt.boxTemplate=(opt.boxTemplate||"<span class='"+opt.boxClass+"'>&nbsp</span>");if(opt.horizontal){opt.batchTemplate=(opt.batchTemplate||"<td></td>");$sp=jQuery("<table><tr></tr></table>").addClass(opt.scrollPaneClass);$me.append($sp);offset=batch($sp.find("tr"),offset,opt);_bz();_ab();fnEnd=hEnd;fnScroll=hScroll;}else{opt.batchTemplate=(opt.batchTemplate||"<span></span>");$sp=jQuery("<div></div>").addClass(opt.scrollPaneClass);$me.append($sp);offset=batch($sp,offset,opt);_bz();_ab();fnEnd=vEnd;fnScroll=vScroll;}setTimeout(monEnd,opt.endDelay);if(typeof opt.updateBatch=='function'){setTimeout(monScroll,opt.scrollDelay);}function _css(){if(opt.horizontal){$me.css({"overflow-x":"auto","overflow-y":"hidden"});}else{$me.css({"overflow-x":"hidden","overflow-y":"auto"});}}function _ab(){var os,b;if(opt.horizontal){os=$me.find('.batch:first').next().offset().left;b=($me.width()/os+1)*os;}else{os=$me.find('.batch:first').next().offset().top;b=($me.height()/os+1)*os;}if("auto"==opt.uBound){opt.uBound=b;}if("auto"==opt.lBound){opt.lBound=-b;}if("auto"==opt.eBound){opt.eBound=b*2;}}function _bz(){$me.scrollTop(0).scrollLeft(0);};function batch($s,o,opt){var $b,i,rp=opt.batchNum;while(rp--){$b=jQuery(opt.batchTemplate).attr({offset:o,len:opt.batchSize}).addClass(opt.batchClass+" "+opt.emptyBatchClass);i=opt.batchSize;while(i--&&opt.maxOffset>o++){$b.append(opt.boxTemplate);}$s.append($b);}return o;};function vScroll(){var so=$me.scrollTop();if(lsp!=so){lsp=so;var co=$me.offset().top;$sp.find('> .'+opt.emptyBatchClass).each(function(i,obj){var $b=jQuery(obj);var p=$b.position().top-co;if(opt.lBound>p||p>opt.uBound){return;}opt.updateBatch($b.removeClass(opt.emptyBatchClass));});}};function hScroll(){var so=$me.scrollLeft();if(lsp!=so){lsp=so;var co=$me.offset().left;$sp.find('tr > .'+opt.emptyBatchClass).each(function(i,obj){var $b=jQuery(obj);var p=$b.position().left-co;if(opt.lBound>p||p>opt.uBound){return;}opt.updateBatch($b.removeClass(opt.emptyBatchClass));});}};function vEnd(){if(ele.scrollTop>0&&ele.scrollHeight-ele.scrollTop<opt.eBound){offset=batch($sp,offset,opt);return 1;}return opt.endDelay;};function hEnd(){if(ele.scrollLeft>0&&ele.scrollWidth-ele.scrollLeft<opt.eBound){offset=batch($sp.find("tr:first"),offset,opt);return 1;}return opt.endDelay;};function monScroll(){fnScroll();setTimeout(monScroll,opt.scrollDelay);};function monEnd(){if(offset<opt.maxOffset){setTimeout(monEnd,fnEnd());}}});};
 14+})(jQuery);
Property changes on: trunk/extensions/Storyboard/tags/Storyboard/jquery.ajaxscroll.min.js
___________________________________________________________________
Added: svn:eol-style
115 + native
Index: trunk/extensions/Storyboard/tags/Storysubmission/Storysubmission_body.php
@@ -18,12 +18,31 @@
1919 // http://www.mediawiki.org/wiki/Manual:Forms
2020 // http://www.mediawiki.org/wiki/Manual:Hooks/UnknownAction
2121 public static function render( $input, $args, $parser, $frame ) {
 22+ wfProfileIn( __METHOD__ );
2223
23 - return <<<END
24 -<form name="storysubmission" action="" method="get">
 24+ global $wgRequest;
 25+
 26+ if ($wgRequest->wasPosted()) {
 27+ $output = $this->doSubmissionAndGetResult();
 28+ } else {
 29+ $output = $this->getFrom();
 30+ }
 31+
 32+ return $output;
 33+
 34+ wfProfileOut( __METHOD__ );
 35+ }
 36+
 37+ private function getFrom() {
 38+ return <<<EOT
 39+<form name="storysubmission" action="#" method="get">
2540
2641 </form>
27 -END;
 42+EOT;
2843 }
2944
 45+ private function doSubmissionAndGetResult() {
 46+
 47+ }
 48+
3049 }
\ No newline at end of file

Comments

#Comment by Catrope (talk | contribs)   22:59, 25 February 2010
+		parent :: __construct( $main, $action );

Since this is a list module, it should pass a parameter prefix, e.g 'st', as the third argument here.

+		// Get a slave db object to do read operations against.
+		$dbr = wfGetDB( DB_SLAVE );

The ApiQueryBase parent class provides a lot of nice functions for doing DB requests so you don't even have to grab a DB object. Example usage is all over the place in trunk/phase3/includes/api/ApiQuery*.php ; the functions themselves (with docs) are in ApiQueryBase.php .

+			'size' => array (
+				ApiBase :: PARAM_DFLT => 5,
+				ApiBase :: PARAM_TYPE => 'integer',
+				ApiBase :: PARAM_MIN => 1,
+				ApiBase :: PARAM_MAX => 100,
+			),				

You probably want to use 'limit' as a param name and 'limit' as type as well. There's dozens of API modules you can copy the 'limit' parameter settings from. Also, continuations (which is presumably what offset is intended to be used for) are customarily done with a parameter called 'continue'; there's also the setContinueEnumParameter() function to help you there. Again, example usage is plentiful in ApiQuery*.php .

+	protected function getExamples() {
+		return array (
+			'api.php?action=stories',
+			'api.php?action=stories&offset=42',
+			'api.php?action=stories&offset=4&size=2',
+		);
+	}

Since this is a list module, and taking into account the remarks above, these examples would look more like api.php?action=query&list=stories&stlimit=2&stcontinue=42.

+$egStoryboardMaxStoryLen = 1000;
+$egStoryboardMinStoryLen = 10;
+
+$egStoryboardWidth = '80%';
+$egStoryboardHieght = 400;
+
+$egStoryboardBatchSize = 5;
+$egStoryboardBatchAmount = 2;

Please document these variables so users know what they mean and can tweak them in their LocalSettings.php . Also, you misspelled Height here.

+++ trunk/extensions/Storyboard/tags/Storyboard/jquery.ajaxscroll.min.js	(revision 62977)

Please also add an unminified version of this plugin, along with a config variable to switch between using the original and minified version. This makes debugging and changing the plugin easier. I can write you a Makefile that'll let you autogenerate the minified file from a changed original file if you want, just commit the plain file to SVN and poke me.

Status & tagging log