r42521 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r42520‎ | r42521 | r42522 >
Date:00:10, 25 October 2008
Author:brion
Status:old (Comments)
Tags:
Comment:
Revert r42514 for now "(bug 16073) * Use onclick handler for expand/collapse in enhanced recentchanges."

A few issues I noted:
* The expand/contract icons no longer have regular link behavior (eg no hand icon, can't be reached by keyboard tabbing)
* It looks like the stuff-to-be-hidden doesn't get hidden until after the </body>, which feels a little sketchy to me. On long lists and slow connections you may see odd behavior with the items being shown expanded, then suddenly hiding when things reach the end. Adding the style immediately in the JS instead of waiting for the body load completion should avoid that
* mw-rc-jshidden class seems to be applied to things that shouldn't have it sincec they are explicitly given display:none?
* Instead of style='display:none' etc, consider using clear classes for expanded and hidden modes, then switch the classes instead of the styles in the JS
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/ChangesList.php (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/skins/common/enhancedchanges.js (deleted) (history)
  • /trunk/phase3/skins/common/wikibits.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/enhancedchanges.js
——@@ -1,33 +0,0 @@
2 -/*
3 - JavaScript file for enhanced recentchanges
4 - */
5 -
6 -/*
7 - * onload hook to add the CSS to hide parts that should be collapsed
8 - *
9 - * We do this with JS so everything will be expanded by default
10 - * if JS is disabled
11 - */
12 -addOnloadHook(function () {
13 - var css = ".mw-rc-jshidden { display:none; }";
14 - appendCSS(css);
15 -});
16 -
17 -/*
18 - * Switch an RC line between hidden/shown
19 - * @param int idNumber : the id number of the RC group
20 -*/
21 -function toggleVisibility(idNumber) {
22 - var openarrow = document.getElementById("mw-rc-openarrow-"+idNumber);
23 - var closearrow = document.getElementById("mw-rc-closearrow-"+idNumber);
24 - var subentries = document.getElementById("mw-rc-subentries-"+idNumber);
25 - if (openarrow.style.display == 'inline') {
26 - openarrow.style.display = 'none';
27 - closearrow.style.display = 'inline';
28 - subentries.style.display = 'block';
29 - } else {
30 - openarrow.style.display = 'inline';
31 - closearrow.style.display = 'none';
32 - subentries.style.display = 'none';
33 - }
34 -}
Index: trunk/phase3/skins/common/wikibits.js
——@@ -104,6 +104,22 @@
105105 }
106106 }
107107
 108+// for enhanced RecentChanges
 109+function toggleVisibility(_levelId, _otherId, _linkId) {
 110+ var thisLevel = document.getElementById(_levelId);
 111+ var otherLevel = document.getElementById(_otherId);
 112+ var linkLevel = document.getElementById(_linkId);
 113+ if (thisLevel.style.display == 'none') {
 114+ thisLevel.style.display = 'block';
 115+ otherLevel.style.display = 'none';
 116+ linkLevel.style.display = 'inline';
 117+ } else {
 118+ thisLevel.style.display = 'none';
 119+ otherLevel.style.display = 'inline';
 120+ linkLevel.style.display = 'none';
 121+ }
 122+}
 123+
108124 function showTocToggle() {
109125 if (document.createTextNode) {
110126 // Uses DOM calls to avoid document.write + XHTML issues
Index: trunk/phase3/includes/ChangesList.php
——@@ -388,24 +388,7 @@
389389 * Generate a list of changes using an Enhanced system (use javascript).
390390 */
391391 class EnhancedChangesList extends ChangesList {
392 -
393392 /**
394 - * Add the JavaScript file for enhanced changeslist
395 - * @ return string
396 - */
397 - public function beginRecentChangesList() {
398 - global $wgStylePath, $wgStyleVersion;
399 - $this->rc_cache = array();
400 - $this->rcMoveIndex = 0;
401 - $this->rcCacheIndex = 0;
402 - $this->lastdate = '';
403 - $this->rclistOpen = false;
404 - $script = Xml::tags( 'script', array(
405 - 'type' => 'text/javascript',
406 - 'src' => $wgStylePath . "/common/enhancedchanges.js?$wgStyleVersion" ), '' );
407 - return $script;
408 - }
409 - /**
410393 * Format a line for enhanced recentchange (aka with javascript and block of lines).
411394 */
412395 public function recentChangesLine( &$baseRC, $watched = false ) {
——@@ -613,12 +596,13 @@
614597
615598 $users = ' <span class="changedby">[' . implode( $this->message['semicolon-separator'], $users ) . ']</span>';
616599
617 - # ID for JS visibility toggle
618 - $jsid = $this->rcCacheIndex;
619 -
620 - $toggleLink = "onclick='toggleVisibility($jsid)'";
621 - $tl = "<span id='mw-rc-openarrow-$jsid' style='display:inline;' $toggleLink>" . $this->sideArrow() . "</span>";
622 - $tl .= "<span id='mw-rc-closearrow-$jsid' style='display:none;' class='mw-rc-jshidden' $toggleLink>" . $this->downArrow() . "</span>";
 600+ # Arrow
 601+ $rci = 'RCI'.$this->rcCacheIndex;
 602+ $rcl = 'RCL'.$this->rcCacheIndex;
 603+ $rcm = 'RCM'.$this->rcCacheIndex;
 604+ $toggleLink = "javascript:toggleVisibility('$rci','$rcm','$rcl')";
 605+ $tl = '<span id="'.$rcm.'"><a href="'.$toggleLink.'">' . $this->sideArrow() . '</a></span>';
 606+ $tl .= '<span id="'.$rcl.'" style="display:none"><a href="'.$toggleLink.'">' . $this->downArrow() . '</a></span>';
623607 $r .= '<td valign="top" style="white-space: nowrap"><tt>'.$tl.'&nbsp;';
624608
625609 # Main line
——@@ -696,7 +680,7 @@
697681 $r .= "</td></tr></table>\n";
698682
699683 # Sub-entries
700 - $r .= '<div id="mw-rc-subentries-'.$jsid.'" class="mw-rc-jshidden"><table cellpadding="0" cellspacing="0" border="0" style="background: none">';
 684+ $r .= '<div id="'.$rci.'" style="display:none;"><table cellpadding="0" cellspacing="0" border="0" style="background: none">';
701685 foreach( $block as $rcObj ) {
702686 # Get rc_xxxx variables
703687 // FIXME: Would be good to replace this extract() call with something that explicitly initializes local variables.
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -1385,7 +1385,7 @@
13861386 * to ensure that client-side caches don't keep obsolete copies of global
13871387 * styles.
13881388 */
1389 -$wgStyleVersion = '183';
 1389+$wgStyleVersion = '182';
13901390
13911391
13921392 # Server-side caching:
Index: trunk/phase3/RELEASE-NOTES
——@@ -283,8 +283,6 @@
284284 * (bug 14609) User's namespaces to be searched default not updated after adding new namespace
285285 * Purge form uses valid XHTML and (bug 8992) uses $wgRequest instead of $_SERVER
286286 * (bug 12764) Special:LonelyPages shows transcluded pages
287 -* (bug 16073) Enhanced RecentChanges uses onclick handler with better fallback if
288 - JavaScript is disabled.
289287
290288 === API changes in 1.14 ===
291289

Follow-up revisions

RevisionCommit summaryAuthorDate
r42528Re-committing r42514:...mrzman02:27, 25 October 2008

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r42514(bug 16073) * Use onclick handler for expand/collapse in enhanced recentchanges....mrzman23:10, 24 October 2008

Comments

#Comment by Mr.Z-man (talk | contribs)   02:33, 25 October 2008

Re-committed with changes in r42528

Status & tagging log