r86742 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r86741‎ | r86742 | r86743 >
Date:23:05, 22 April 2011
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Added more functionality to the ArticleFeedback special page.
Modified paths:
  • /trunk/extensions/ArticleFeedback/ArticleFeedback.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedback/SpecialArticleFeedback.php (modified) (history)
  • /trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.dashboard.css (added) (history)
  • /trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.dashboard.js (added) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedback/SpecialArticleFeedback.php
@@ -17,20 +17,108 @@
1818 public function execute( $par ) {
1919 global $wgUser, $wgOut, $wgRequest;
2020
 21+ $wgOut->addModules( 'ext.articleFeedback.dashboard' );
2122 $this->setHeaders();
 23+ $this->renderDailyHighsAndLows();
 24+ $this->renderWeeklyMostChanged();
 25+ $this->renderRecentLows();
 26+ }
2227
23 - $dailyHighsAndLows = $this->getDailyHighsAndLows();
24 - $weeklyMostChanged = $this->getWeeklyMostChanged();
25 - $recentLows = $this->getRecentLows();
 28+ /* Protected Methods */
2629
27 - // TODO: Use lists to generate tables
 30+ /**
 31+ * Returns an HTML table containing data from a given two dimensional array.
 32+ *
 33+ * @param $headings Array: List of rows, each a list of columns (values will be escaped)
 34+ * @param $rows Array: List of rows, each a list of columns (values will not be escaped)
 35+ * @param $attribs Array: List of HTML attributes to apply to the table
 36+ * @return String: HTML table code
 37+ */
 38+ protected function renderTable( array $headings, array $rows, $id = null ) {
 39+ global $wgOut;
2840
29 - $wgOut->addHtml( '<div class="articleFeedback-dashboard"><h2>Hello dashboard!</h2></div>' );
 41+ $table = Html::openElement( 'table', array(
 42+ 'class' => 'articleFeedback-table sortable', 'id' => $id
 43+ ) );
 44+ // Head
 45+ $table .= Html::openElement( 'thead' );
 46+ $table .= Html::openElement( 'tr' );
 47+ foreach ( $headings as $heading ) {
 48+ $table .= Html::element( 'th', array(), $heading );
 49+ }
 50+ $table .= Html::closeElement( 'tr' );
 51+ $table .= Html::closeElement( 'thead' );
 52+ // Body
 53+ $table .= Html::openElement( 'tbody' );
 54+ foreach ( $rows as $row ) {
 55+ $table .= Html::openElement( 'tr' );
 56+ foreach ( $row as $column ) {
 57+ $table .= Html::openElement( 'td' );
 58+ $table .= $column;
 59+ $table .= Html::closeElement( 'td' );
 60+ }
 61+ $table .= Html::closeElement( 'tr' );
 62+ }
 63+ $table .= Html::closeElement( 'tbody' );
 64+ $table .= Html::closeElement( 'table' );
 65+ $wgOut->addHtml( $table );
3066 }
3167
32 - /* Protected Methods */
 68+ /**
 69+ * Renders daily highs and lows
 70+ *
 71+ * @return String: HTML table of daily highs and lows
 72+ */
 73+ protected function renderDailyHighsAndLows() {
 74+ global $wgOut;
3375
 76+ $data = $this->getDailyHighsAndLows();
 77+
 78+ $wgOut->addHtml( Html::element( 'h2', array(), 'Daily Highs and Lows' ) );
 79+ $this->renderTable(
 80+ array( 'Article Title', 'Ratings', 'Average' ),
 81+ array( /* rendered data */ ),
 82+ 'articleFeedback-table-highsandlows'
 83+ );
 84+ }
 85+
3486 /**
 87+ * Renders weekly most changed
 88+ *
 89+ * @return String: HTML table of weekly most changed
 90+ */
 91+ protected function renderWeeklyMostChanged() {
 92+ global $wgOut;
 93+
 94+ $data = $this->getWeeklyMostChanged();
 95+
 96+ $wgOut->addHtml( Html::element( 'h2', array(), 'Weekly Most Changed' ) );
 97+ $this->renderTable(
 98+ array( 'Article Title', 'Category', 'Difference' ),
 99+ array( /* rendered data */ ),
 100+ 'articleFeedback-table-weeklymostchanged'
 101+ );
 102+ }
 103+
 104+ /**
 105+ * Renders recent lows
 106+ *
 107+ * @return String: HTML table of recent lows
 108+ */
 109+ protected function renderRecentLows() {
 110+ global $wgOut;
 111+
 112+ $data = $this->getRecentLows();
 113+
 114+ $wgOut->addHtml( Html::element( 'h2', array(), 'Recent Lows' ) );
 115+ $this->renderTable(
 116+ array( 'Article Title', 'Category', 'Rating' ),
 117+ array( /* rendered data */ ),
 118+ 'articleFeedback-table-recentlows'
 119+ );
 120+ }
 121+
 122+ /**
35123 * Gets a list of articles which were rated exceptionally high or low.
36124 *
37125 * - Based on average of all rating categories
Index: trunk/extensions/ArticleFeedback/ArticleFeedback.hooks.php
@@ -52,6 +52,10 @@
5353 'jquery.clickTracking',
5454 ),
5555 ),
 56+ 'ext.articleFeedback.dashboard' => array(
 57+ 'scripts' => 'ext.articleFeedback/ext.articleFeedback.dashboard.js',
 58+ 'styles' => 'ext.articleFeedback/ext.articleFeedback.dashboard.css',
 59+ ),
5660 'jquery.articleFeedback' => array(
5761 'scripts' => 'jquery.articleFeedback/jquery.articleFeedback.js',
5862 'styles' => 'jquery.articleFeedback/jquery.articleFeedback.css',
Index: trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.dashboard.css
@@ -0,0 +1,8 @@
 2+.articleFeedback-table {
 3+ border: solid 1px #cccccc;
 4+}
 5+
 6+.articleFeedback-table th {
 7+ text-align: left;
 8+ border-bottom: solid 1px #eeeeee;
 9+}
\ No newline at end of file
Property changes on: trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.dashboard.css
___________________________________________________________________
Added: svn:eol-style
110 + native
Added: svn:mime-type
211 + text/plain
Index: trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.dashboard.js
Property changes on: trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.dashboard.js
___________________________________________________________________
Added: svn:eol-style
312 + native
Added: svn:mime-type
413 + text/plain

Follow-up revisions

RevisionCommit summaryAuthorDate
r86979Using Html::rawElement rather than openElement and closeElement as per commen...tparscal19:58, 26 April 2011
r86991Finished rendering methods, added styling, and i18n/L10n (which also resolves...tparscal22:20, 26 April 2011

Comments

#Comment by Catrope (talk | contribs)   16:19, 26 April 2011
+				$table .= Html::openElement( 'td' );
+				$table .= $column;
+				$table .= Html::closeElement( 'td' );

There's something like rawElement() (don't recall the exact name) to express this in simpler terms.

+			array( 'Article Title', 'Ratings', 'Average' ),

Missing i18n (there are other occurrences too). I know this is still skeleton-y and unfinished, but still.

OK otherwise

#Comment by Trevor Parscal (WMF) (talk | contribs)   19:59, 26 April 2011

1. Yup, Html::rawElement is the name - fixed this up in r86979 2. We aren't even sure which columns we will have - so, I didn't bother with i18n yet.

#Comment by Catrope (talk | contribs)   20:19, 26 April 2011

OK, no worries then, just making sure you're aware.

Status & tagging log