r86056 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r86055‎ | r86056 | r86057 >
Date:16:06, 14 April 2011
Author:diebuche
Status:ok (Comments)
Tags:
Comment:
Followup to r86047: Rewrite the radio-button updater & move everything to action.history. Also move to fixCompare version by Helder, since HTMLdiff is scrapped
Modified paths:
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/resources/mediawiki.action/mediawiki.action.history.js (modified) (history)
  • /trunk/phase3/skins/common/history.js (deleted) (history)

Diff [purge]

Index: trunk/phase3/skins/common/history.js
——@@ -1,159 +0,0 @@
2 -window.historyRadios = function( parent ) {
3 - var inputs = parent.getElementsByTagName( 'input' );
4 - var radios = [],
5 - i = 0;
6 - for ( i = 0; i < inputs.length; i++ ) {
7 - if ( inputs[i].name == 'diff' || inputs[i].name == 'oldid' ) {
8 - radios[radios.length] = inputs[i];
9 - }
10 - }
11 - return radios;
12 -};
13 -
14 -// check selection and tweak visibility/class onclick
15 -window.diffcheck = function() {
16 - var dli = false, // the li where the diff radio is checked
17 - oli = false, // the li where the oldid radio is checked
18 - i = 0;
19 - var hf = document.getElementById( 'pagehistory' );
20 - if ( !hf ) {
21 - return true;
22 - }
23 - var lis = hf.getElementsByTagName( 'li' );
24 - for ( i = 0; i < lis.length; i++ ) {
25 - var inputs = historyRadios( lis[i] );
26 - if ( inputs[1] && inputs[0] ) {
27 - if ( inputs[1].checked || inputs[0].checked ) { // this row has a checked radio button
28 - if ( inputs[1].checked && inputs[0].checked && inputs[0].value == inputs[1].value ) {
29 - return false;
30 - }
31 - if ( oli ) { // it's the second checked radio
32 - if ( inputs[1].checked ) {
33 - if ( typeof oli.className != 'undefined' ) {
34 - oli.classNameOriginal = oli.className.replace( 'selected', '' );
35 - } else {
36 - oli.classNameOriginal = '';
37 - }
38 -
39 - oli.className = 'selected ' + oli.classNameOriginal;
40 - return false;
41 - }
42 - } else if ( inputs[0].checked ) {
43 - return false;
44 - }
45 - if ( inputs[0].checked ) {
46 - dli = lis[i];
47 - }
48 - if ( !oli ) {
49 - inputs[0].style.visibility = 'hidden';
50 - }
51 - if ( dli ) {
52 - inputs[1].style.visibility = 'hidden';
53 - }
54 - if ( (typeof lis[i].className) != 'undefined') {
55 - lis[i].classNameOriginal = lis[i].className.replace( 'selected', '' );
56 - } else {
57 - lis[i].classNameOriginal = '';
58 - }
59 -
60 - lis[i].className = 'selected ' + lis[i].classNameOriginal;
61 - oli = lis[i];
62 - } else { // no radio is checked in this row
63 - if ( !oli ) {
64 - inputs[0].style.visibility = 'hidden';
65 - } else {
66 - inputs[0].style.visibility = 'visible';
67 - }
68 - if ( dli ) {
69 - inputs[1].style.visibility = 'hidden';
70 - } else {
71 - inputs[1].style.visibility = 'visible';
72 - }
73 - if ( typeof lis[i].classNameOriginal != 'undefined' ) {
74 - lis[i].className = lis[i].classNameOriginal;
75 - }
76 - }
77 - }
78 - }
79 - return true;
80 -};
81 -
82 -//update the compare link as you select radio buttons
83 -window.updateCompare = function () {
84 - var hf = compareLink.$form.get(0);
85 - var oldInd = -1;
86 - var i = 0;
87 - while(oldInd == -1 & i<hf.oldid.length) {
88 - if(hf.oldid[i].checked){
89 - oldInd=i;
90 - }
91 - i++;
92 - }
93 - var diffInd=-1;
94 - var j=0;
95 - while(diffInd==-1 & j<hf.diff.length) {
96 - if(hf.diff[j].checked){
97 - diffInd=j;
98 - }
99 - j++;
100 - }
101 - var wikiLinkURL = wgServer + wgScript + "?title=" + encodeURIComponent(hf.title.value)
102 - + "&diff=" + hf.diff[diffInd].value + "&oldid=" + hf.oldid[oldInd].value;
103 - compareLink.wikiTop.attr("href", wikiLinkURL);
104 - compareLink.wikiEnd.attr("href", wikiLinkURL);
105 -
106 - if(compareLink.htmlDiffs)
107 - {
108 - var htmlLinkURL = wgServer + wgScript + "?title=" + encodeURIComponent(hf.title.value)
109 - + "&htmldiff=" + compareLink.htmlDiffButtonTxt
110 - + "&diff=" + hf.diff[diffInd].value + "&oldid=" + hf.oldid[oldInd].value;
111 - compareLink.htmlTop.attr("href", htmlLinkURL);
112 - compareLink.htmlEnd.attr("href", htmlLinkURL);
113 - }
114 -};
115 -
116 -//change the button to a link where possible
117 -window.fixCompare = function () {
118 - window.compareLink = {};
119 - var doneHtml = false;
120 - var doneWiki = false;
121 - compareLink.htmlDiffs = false;
122 -
123 - compareLink.$form = $("#mw-history-compare");
124 - var hf = compareLink.$form.get(0);
125 -
126 - var $buttons = $('input.historysubmit');
127 -
128 - if (!compareLink.$form.length || !$buttons.length) return;
129 -
130 - $buttons.each(function() {
131 - if(this.name == "htmldiff") {
132 - if (doneHtml) return true;
133 - doneHtml = true;
134 - var url = wgServer + wgScript + "?title=" + encodeURIComponent(hf.title.value)
135 - + "&htmldiff=" + this.value + "&diff=" + hf.diff[0].value + "&oldid=" + hf.oldid[1].value;
136 - compareLink.htmlDiffs = true;
137 - } else {
138 - if (doneWiki) return true;
139 - doneWiki = true;
140 - var url = wgServer + wgScript + "?title=" + encodeURIComponent(hf.title.value)
141 - + "&diff=" + hf.diff[0].value + "&oldid=" + hf.oldid[1].value;
142 - }
143 - var $linkTop = $("<a href='" + url + "' class='historycomparelink ui-button'>" + this.value + "</a>").button();
144 - compareLink.$form.before($linkTop);
145 - $linkEnd = $linkTop.clone();
146 - compareLink.$form.append($linkEnd);
147 -
148 - if(this.name == "htmldiff") {
149 - compareLink.htmlTop = $linkTop;
150 - compareLink.htmlEnd = $linkEnd;
151 - } else {
152 - compareLink.wikiTop = $linkTop;
153 - compareLink.wikiEnd = $linkEnd;
154 - }
155 - });
156 - $buttons.hide();
157 -
158 - $("#pagehistory").change(function() {window.updateCompare()});
159 -};
160 -
Index: trunk/phase3/resources/mediawiki.action/mediawiki.action.history.js
——@@ -2,8 +2,78 @@
33 * JavaScript for History action
44 */
55 jQuery( function( $ ) {
6 - // Replaces histrowinit
7 - $( '#pagehistory li input[name="diff"], #pagehistory li input[name="oldid"]' ).click( diffcheck );
8 - diffcheck();
9 - window.fixCompare();
 6+ var $lis = $( 'ul#pagehistory li' );
 7+ var updateDiffRadios = function() {
 8+ var diffLi = false, // the li where the diff radio is checked
 9+ oldLi = false; // the li where the oldid radio is checked
 10+
 11+ if ( !$lis.length ) {
 12+ return true;
 13+ }
 14+ $lis.removeClass( 'selected' );
 15+ $lis.each( function() {
 16+ var $this = $(this);
 17+ var $inputs = $this.find( 'input[type=radio]' );
 18+ if ( $inputs.length !== 2 ) {
 19+ return true;
 20+ }
 21+
 22+ // this row has a checked radio button
 23+ if ( $inputs.get(0).checked ) {
 24+ oldLi = $this;
 25+ $this.addClass( 'selected' );
 26+ $inputs.eq(0).css( 'visibility', 'visible' );
 27+ $inputs.eq(1).css( 'visibility', 'hidden' );
 28+ } else if ( $inputs.get(1).checked ) {
 29+ diffLi = $this;
 30+ $this.addClass( 'selected' );
 31+ $inputs.eq(0).css( 'visibility', 'hidden' );
 32+ $inputs.eq(1).css( 'visibility', 'visible' );
 33+ } else {
 34+ // no radio is checked in this row
 35+ if ( diffLi && oldLi ) {
 36+ // We're below the selected radios
 37+ $inputs.eq(0).css( 'visibility', 'visible' );
 38+ $inputs.eq(1).css( 'visibility', 'hidden' );
 39+ } else if ( diffLi ) {
 40+ // We're between the selected radios
 41+ $inputs.eq(0).css( 'visibility', 'visible' );
 42+ $inputs.eq(1).css( 'visibility', 'visible' );
 43+ } else {
 44+ // We're above the selected radios
 45+ $inputs.eq(1).css( 'visibility', 'visible' );
 46+ $inputs.eq(0).css( 'visibility', 'hidden' );
 47+ }
 48+ }
 49+ });
 50+ return true;
 51+ };
 52+
 53+ var fixCompare = function () {
 54+ var $histForm = $( '#mw-history-compare' ),
 55+ $diffList = $( '#pagehistory' ),
 56+ buttonText = $histForm.find( 'input.historysubmit' ).remove().first().val(),
 57+ $compareLink = $( '<a></a>', {
 58+ 'class': 'compare-link',
 59+ 'text': buttonText
 60+ }).button();
 61+ $histForm.prepend( $compareLink )
 62+ .append( $compareLink.clone() );
 63+ function updateCompare() {
 64+ var $radio = $histForm.find( 'input[type=radio]:checked' );
 65+ var genLink = mw.config.get( 'wgScript' )
 66+ + '?title=' + mw.util.wikiUrlencode( mw.config.get( 'wgPageName' ) )
 67+ + '&diff=' + $radio.eq(0).val()
 68+ + '&oldid=' + $radio.eq(1).val();
 69+ $( '.compare-link' ).each( function() {
 70+ $(this).attr('href', genLink);
 71+ });
 72+ }
 73+ updateCompare();
 74+ $diffList.change( updateCompare );
 75+ };
 76+
 77+ $( '#pagehistory li input[name="diff"], #pagehistory li input[name="oldid"]' ).click( function() { updateDiffRadios(); } );
 78+ fixCompare();
 79+ updateDiffRadios();
1080 });
\ No newline at end of file
Index: trunk/phase3/resources/Resources.php
——@@ -403,7 +403,7 @@
404404 ),
405405 'mediawiki.action.history' => array(
406406 'scripts' => 'resources/mediawiki.action/mediawiki.action.history.js',
407 - 'dependencies' => 'mediawiki.legacy.history',
 407+ 'dependencies' => 'jquery.ui.button',
408408 'group' => 'mediawiki.action.history',
409409 ),
410410 'mediawiki.action.edit' => array(
——@@ -545,13 +545,6 @@
546546 'localBasePath' => "{$GLOBALS['IP']}/skins",
547547 'dependencies' => 'mediawiki.legacy.wikibits',
548548 ),
549 - 'mediawiki.legacy.history' => array(
550 - 'scripts' => 'common/history.js',
551 - 'group' => 'mediawiki.action.history',
552 - 'remoteBasePath' => $GLOBALS['wgStylePath'],
553 - 'localBasePath' => "{$GLOBALS['IP']}/skins",
554 - 'dependencies' => array( 'mediawiki.legacy.wikibits', 'jquery.ui.button' ),
555 - ),
556549 'mediawiki.legacy.IEFixes' => array(
557550 'scripts' => 'common/IEFixes.js',
558551 'remoteBasePath' => $GLOBALS['wgStylePath'],

Follow-up revisions

RevisionCommit summaryAuthorDate
r86480Followup to r86056 per CR: Remove unneeded function wrappingdiebuche15:22, 20 April 2011
r86483Followup to r86056 per CR: No need to keep jQuery object, just set it to bool...diebuche15:30, 20 April 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r86047History: Adding JS that converts buttons to links for better UX. Patch based...diebuche13:18, 14 April 2011

Comments

#Comment by Krinkle (talk | contribs)   12:30, 17 April 2011

Nice work. Rewrite from legacy to mw-lib for the history looks pretty good here! 

+			var $this = $(this);
+		var diffLi = false, // the li where the diff radio is checked
+			oldLi = false; // the li where the oldid radio is checked

+				oldLi = $this;
+				diffLi = $this;

+				if ( diffLi && oldLi ) {
+	 			} else if ( diffLi ) {

jQuery objects don't return boolean false when they don't contain elements, use .length instead. 


+ [..] .click( function() { updateDiffRadios(); } );

No need to create an anonymous function, you can pass the function directly by reference.

#Comment by Catrope (talk | contribs)   12:32, 17 April 2011

If you need to create an empty jQuery object, you can do so with var foo = $( [] );

#Comment by Aaron Schulz (talk | contribs)   01:05, 7 May 2011

The upper right "show/hide revs" button is mal-aligned (FF4 at least).

#Comment by DieBuche (talk | contribs)   12:35, 6 July 2011

Respective part is reverted in r91547

Status & tagging log