r61990 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r61989‎ | r61990 | r61991 >
Date:21:07, 4 February 2010
Author:ialex
Status:ok (Comments)
Tags:
Comment:
Per Tim, fix r59949: use CSS padding (margin doesn't seem to work on Firefox and Safari) instead of fake empty columns, also added text-align:center
Modified paths:
  • /trunk/phase3/includes/specials/SpecialRevisiondelete.php (modified) (history)
  • /trunk/phase3/skins/common/shared.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/shared.css
@@ -547,6 +547,11 @@
548548 visibility: hidden;
549549 }
550550
 551+td.mw-revdel-checkbox, th.mw-revdel-checkbox {
 552+ padding-right: 10px;
 553+ text-align: center;
 554+}
 555+
551556 /* feed links */
552557 a.feedlink {
553558 background: url("images/feed-icon.png") center left no-repeat;
Index: trunk/phase3/includes/specials/SpecialRevisiondelete.php
@@ -441,7 +441,7 @@
442442 */
443443 protected function buildCheckBoxes() {
444444 global $wgRequest;
445 -
 445+
446446 $html = '<table>';
447447 // If there is just one item, use checkboxes
448448 $list = $this->getList();
@@ -462,10 +462,10 @@
463463 // Otherwise, use tri-state radios
464464 } else {
465465 $html .= '<tr>';
466 - $html .= '<th>'.wfMsgHtml('revdelete-radio-same').'</th><th>&nbsp;</th>';
467 - $html .= '<th>'.wfMsgHtml('revdelete-radio-unset').'</th><th>&nbsp;</th>';
468 - $html .= '<th>'.wfMsgHtml('revdelete-radio-set').'</th><th>&nbsp;</th>';
469 - $html .= '<th></th></tr>';
 466+ $html .= '<th class="mw-revdel-checkbox">'.wfMsgHtml('revdelete-radio-same').'</th>';
 467+ $html .= '<th class="mw-revdel-checkbox">'.wfMsgHtml('revdelete-radio-unset').'</th>';
 468+ $html .= '<th class="mw-revdel-checkbox">'.wfMsgHtml('revdelete-radio-set').'</th>';
 469+ $html .= "<th></th></tr>\n";
470470 foreach( $this->checks as $item ) {
471471 list( $message, $name, $field ) = $item;
472472 // If there are several items, use third state by default...
@@ -474,9 +474,9 @@
475475 } else {
476476 $selected = -1; // use existing field
477477 }
478 - $line = '<td>' . Xml::radio( $name, -1, $selected == -1 ) . '</td><td>&nbsp;</td>';
479 - $line .= '<td>' . Xml::radio( $name, 0, $selected == 0 ) . '</td><td>&nbsp;</td>';
480 - $line .= '<td>' . Xml::radio( $name, 1, $selected == 1 ) . '</td><td>&nbsp;</td>';
 478+ $line = '<td class="mw-revdel-checkbox">' . Xml::radio( $name, -1, $selected == -1 ) . '</td>';
 479+ $line .= '<td class="mw-revdel-checkbox">' . Xml::radio( $name, 0, $selected == 0 ) . '</td>';
 480+ $line .= '<td class="mw-revdel-checkbox">' . Xml::radio( $name, 1, $selected == 1 ) . '</td>';
481481 $label = wfMsgHtml($message);
482482 if( $field == Revision::DELETED_RESTRICTED ) {
483483 $label = "<b>$label</b>";

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r59949(bug 20928) Added tri-state form for RevisionDelete when there are multiple i...aaron06:57, 11 December 2009

Comments

#Comment by Simetrical (talk | contribs)   21:28, 5 February 2010

Margin properties don't apply to table cells. Where the border of one cell ends, the border of the next begins, unless it's at the edge of the table, in which case it's the margin of the table. (Or if borders are collapsing, then the next cell's padding begins where the border ends.) If nonzero margin were accepted on table cells, you'd have gaps between the borders of adjacent table cells, which would be kind of odd. Padding goes inside the border, so it makes sense on anything that can contain children other than table cells or rows.

#Comment by Tim Starling (talk | contribs)   03:48, 19 February 2010

Thanks Alex.

Status & tagging log