r109726 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r109725‎ | r109726 | r109727 >
Date:07:10, 22 January 2012
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
[ApiSandbox] front-end makeover
* Use bolded table headings
* jQuery UI buttons
* Row highlighting and more padding/spacing, less clutter
* moving main buttons out of the table to the top right above the form
* keeping submit button below the form
* duplicate submit button above the form
* automatically scroll down to the response after a request is processed
Modified paths:
  • /trunk/extensions/ApiSandbox/ApiSandbox.i18n.php (modified) (history)
  • /trunk/extensions/ApiSandbox/ApiSandbox.php (modified) (history)
  • /trunk/extensions/ApiSandbox/SpecialApiSandbox.php (modified) (history)
  • /trunk/extensions/ApiSandbox/ext.apiSandbox.css (modified) (history)
  • /trunk/extensions/ApiSandbox/ext.apiSandbox.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ApiSandbox/SpecialApiSandbox.php
——@@ -39,12 +39,12 @@
4040 . $this->openFieldset( 'result' )
4141 . '<table class="api-sandbox-result-container"><tbody>
4242 '
43 - . '<tr><td class="api-sandbox-result-label"><label for="api-sandbox-url">'
44 - . wfMessage( 'apisb-request-url' )->parse() . '</label></td>'
 43+ . '<tr><th class="api-sandbox-result-label"><label for="api-sandbox-url">'
 44+ . wfMessage( 'apisb-request-url' )->parse() . '</label></th>'
4545 . '<td><input id="api-sandbox-url" readonly="readonly" /></td></tr>
4646 '
47 - . '<tr id="api-sandbox-post-row"><td class="api-sandbox-result-label"><label for="api-sandbox-post">'
48 - . wfMessage( 'apisb-request-post' )->parse() . '</label></td>'
 47+ . '<tr id="api-sandbox-post-row"><th class="api-sandbox-result-label"><label for="api-sandbox-post">'
 48+ . wfMessage( 'apisb-request-post' )->parse() . '</label></th>'
4949 . '<td><input id="api-sandbox-post" readonly="readonly" /></td></tr>
5050 '
5151 . '<tr><td colspan="2"><div id="api-sandbox-output"></div></td></tr>'
——@@ -78,21 +78,29 @@
7979 $this->getQueryModules( 'meta' )
8080 );
8181
82 - $s = '<table class="api-sandbox-options">
83 -<tbody>
84 -';
85 - $s .= '<tr><td class="api-sandbox-label"><label for="api-sandbox-format"><code>format=</code></label></td><td class="api-sandbox-value">'
86 - . self::getSelect( 'format', $formats, 'json' )
87 - . '</td><td>' . $this->getButtonsBox() . '</td></tr>
88 -';
89 - $s .= '<tr><td class="api-sandbox-label"><label for="api-sandbox-action"><code>action=</code></label></td><td class="api-sandbox-value">'
90 - . self::getSelect( 'action', $modules )
91 - . '</td><td id="api-sandbox-help" rowspan="2"></td></tr>
92 -';
93 - $s .= '<tr id="api-sandbox-query-row" style="display: none"><td class="api-sandbox-label">'
94 - . '</td><td class="api-sandbox-value">'
95 - . self::getSelect( 'query', $queryModules )
96 - . '</td></tr>
 82+ $s = '<div id="api-sandbox-buttons"></div>';
 83+ $s .= '<div id="api-sandbox-examples" style="display: none;"></div>';
 84+ $s .= '
 85+<table class="api-sandbox-options">
 86+ <tbody>
 87+ <tr>
 88+ <th><label for="api-sandbox-format">Format</label></th>
 89+ <th><label for="api-sandbox-action">Action</label></th>
 90+ <th class="api-sandbox-docs-col">Documentation</th>
 91+ </tr>
 92+ <tr>
 93+ <td>' . self::getSelect( 'format', $formats, 'json' ) . '</td>
 94+ <td>
 95+ ' . self::getSelect( 'action', $modules ) . '
 96+ <div id="api-sandbox-query-row" style="display: none;">
 97+ ' . self::getSelect( 'query', $queryModules ) . '
 98+ </div>
 99+ </td>
 100+ <td class="api-sandbox-docs-col">
 101+ <span id="api-sandbox-help"></span>
 102+ </td>
 103+ </tr>
 104+ </tbody>
97105 </table>
98106 ';
99107 $s .= '<div id="api-sandbox-main-inputs"></div><div id="api-sandbox-query-inputs" style="display: none"></div>'
——@@ -101,22 +109,9 @@
102110 . $this->openFieldset( 'generator-parameters', array( 'style' => 'display: none;' ) )
103111 . '<div id="api-sandbox-generator-inputs"></div></fieldset>
104112 ';
105 -
106 - $s .= Html::element( 'input',
107 - array(
108 - 'type' => 'submit',
109 - 'id' => 'api-sandbox-submit',
110 - 'value' => wfMessage( 'apisb-submit' )->text(),
111 - 'disabled' => 'disabled',
112 - )
113 - ) . "\n";
114113 return $s;
115114 }
116115
117 - public function getButtonsBox() {
118 - return '<div id="api-sandbox-buttons"><div id="api-sandbox-examples" style="display: none"></div></div>';
119 - }
120 -
121116 /**
122117 * @param $type string
123118 * @return array
Index: trunk/extensions/ApiSandbox/ext.apiSandbox.js
——@@ -7,12 +7,33 @@
88 paramInfo, namespaceOptions,
99 // page elements
1010 $format, $action, $query, $queryRow, $help, $mainContainer, $genericContainer,
11 - $generatorContainer, $queryContainer, $generatorBox, $submit, $requestUrl, $requestPost,
12 - $output, $postRow, $buttonsContainer, $examplesButton, $examplesContent;
 11+ $generatorContainer, $queryContainer, $generatorBox, $form, $submit, $requestUrl, $requestPost,
 12+ $output, $postRow, $buttonsContainer, $examplesButton, $examplesContent, $pageScroll;
1313
1414
1515 /** Local utility functions **/
1616
 17+ // get the first element in a list that is "scrollable"
 18+ // depends on browser and skin (i.e. body or html)
 19+ function getScrollableElement( /* selectors, .. */ ) {
 20+ var i, argLen, el, $el, canScroll;
 21+ for ( i = 0, argLen = arguments.length; i < argLen; i += 1 ) {
 22+ el = arguments[i];
 23+ $el = $(el);
 24+ if ( $el.scrollTop() > 0 ) {
 25+ return el;
 26+ } else {
 27+ $el.scrollTop( 1 );
 28+ canScroll = $el.scrollTop() > 0;
 29+ $el.scrollTop( 0 );
 30+ if ( canScroll ) {
 31+ return el;
 32+ }
 33+ }
 34+ }
 35+ return [];
 36+ }
 37+
1738 function showLoading( $element ) {
1839 $element.html(
1940 mw.html.element( 'img',
——@@ -185,7 +206,7 @@
186207 ).appendTo( $list );
187208 count++;
188209 }
189 - $examplesButton.text( mw.msg( count === 1 ? 'apisb-example' : 'apisb-examples' ) );
 210+ $examplesButton.button( 'option', 'text', mw.msg( count === 1 ? 'apisb-example' : 'apisb-examples' ) );
190211 $list.appendTo( $examplesContent );
191212 if ( count ) {
192213 $examplesButton.show();
——@@ -199,7 +220,7 @@
200221 isQuery = action === 'query';
201222
202223 if ( action === '-' || ( isQuery && query === '-' ) ) {
203 - $submit.prop( 'disabled', true );
 224+ $submit.button( 'option', 'disabled', true );
204225 return;
205226 }
206227 query = query.replace( /^.*=/, '' );
——@@ -212,7 +233,7 @@
213234 getParamInfo( data,
214235 function () {
215236 showLoading( $mainContainer );
216 - $submit.prop( 'disabled', true );
 237+ $submit.button( 'option', 'disabled', true );
217238 $examplesContent.hide();
218239 },
219240 function () {
——@@ -224,11 +245,11 @@
225246 }
226247 mainRequest = new UiBuilder( $mainContainer, info, '' );
227248 mainRequest.setHelp( $help );
228 - $submit.prop( 'disabled', false );
 249+ $submit.button( 'option', 'disabled', false );
229250 updateExamples( info );
230251 },
231252 function () {
232 - $submit.removeAttr( 'disabled' );
 253+ $submit.button( 'option', 'disabled', false );
233254 showLoadError( $mainContainer, 'apisb-load-error' );
234255 $examplesContent.hide();
235256 }
——@@ -309,23 +330,29 @@
310331 * Creates inputs and places them into container
311332 */
312333 createInputs: function () {
313 - var $table, i, length, param, name;
 334+ var $table, $tbody, i, length, param, name;
314335
315 - $table = $( '<table class="api-sandbox-options"></table>' );
 336+ $table = $( '<table class="api-sandbox-params mw-datatable"><thead><tr></tr></thead><tbody></tbody></table>' )
 337+ .find( '> thead > tr' )
 338+ .append( mw.html.element( 'th', { 'class': 'api-sandbox-params-label' }, mw.msg( 'apisb-params-param' ) ) )
 339+ .append( mw.html.element( 'th', { 'class': 'api-sandbox-params-value' }, mw.msg( 'apisb-params-input' ) ) )
 340+ .append( mw.html.element( 'th', {}, mw.msg( 'apisb-params-desc' ) ) )
 341+ .end();
 342+ $tbody = $table.find( '> tbody' )
316343 for ( i = 0, length = this.params.length; i < length; i += 1 ) {
317344 param = this.params[i];
318345 name = this.prefix + param.name;
319346
320347 $( '<tr>' )
321348 .append(
322 - $( '<td class="api-sandbox-label"></td>' )
 349+ $( '<td class="api-sandbox-params-label"></td>' )
323350 .html( mw.html.element( 'label',
324 - { 'for': 'param-' + name }, name + '=' )
 351+ { 'for': 'param-' + name }, name )
325352 )
326353 )
327 - .append( $( '<td class="api-sandbox-value"></td>' ).html( this.input( param, name ) ) )
 354+ .append( $( '<td class="api-sandbox-params-value"></td>' ).html( this.input( param, name ) ) )
328355 .append( $( '<td>' ).html( smartEscape( param.description ) ) )
329 - .appendTo( $table );
 356+ .appendTo( $tbody );
330357 }
331358 this.$container.html( $table );
332359 },
——@@ -335,16 +362,16 @@
336363 * @param $container {jQuery} Container to use
337364 */
338365 setHelp: function ( $container ) {
339 - var desc = smartEscape( this.info.description );
 366+ var linkHtml = '',
 367+ descTxt = smartEscape( this.info.description );
340368 if ( this.info.helpurls && this.info.helpurls[0] ) {
341 - desc = desc.replace( /^([^\r\n\.]*)/,
342 - mw.html.element( 'a', {
343 - 'target': '_blank',
344 - 'href': this.info.helpurls[0]
345 - }, '$1' )
346 - );
 369+ descTxt = descTxt.replace( /^([^\r\n\.]*)/, '$1' ) + ' ';
 370+ linkHtml = mw.html.element( 'a', {
 371+ 'target': '_blank',
 372+ 'href': this.info.helpurls[0]
 373+ }, mw.msg( 'apisb-docs-more' ) );
347374 }
348 - $container.html( desc );
 375+ $container.text( descTxt ).append( mw.msg( 'parentheses', linkHtml ) );
349376 },
350377
351378 input: function ( param, name ) {
——@@ -361,7 +388,8 @@
362389 s = mw.html.element( 'input', {
363390 'class': 'api-sandbox-input',
364391 'id': 'param-' + name,
365 - 'value': value
 392+ 'value': value,
 393+ 'type': 'text'
366394 } );
367395 break;
368396
——@@ -471,21 +499,44 @@
472500 $generatorContainer = $( '#api-sandbox-generator-inputs' );
473501 $queryContainer = $( '#api-sandbox-query-inputs' );
474502 $generatorBox = $( '#api-sandbox-generator-parameters' );
475 - $submit = $( '#api-sandbox-submit' );
476503 $requestUrl = $( '#api-sandbox-url' );
477504 $requestPost = $( '#api-sandbox-post' );
478505 $output = $( '#api-sandbox-output' );
479506 $postRow = $( '#api-sandbox-post-row' );
480507 $buttonsContainer = $( '#api-sandbox-buttons' );
 508+ $examplesContent = $( '#api-sandbox-examples' );
 509+ $pageScroll = $( getScrollableElement( 'body', 'html' ) );
 510+ $form = $( '#api-sandbox-form' );
 511+ $submit = $( '<button>' )
 512+ .text( mw.msg( 'apisb-submit' ) )
 513+ .appendTo( $buttonsContainer );
 514+ $submit = $submit.clone( /*dataAndEvents=*/true, /*deep=*/true )
 515+ .appendTo( '#api-sandbox-parameters' )
 516+ .add( $submit )
 517+ .click( function ( e ) {
 518+ // Avoid triggering other stuff, including the sister-button
 519+ e.stopImmediatePropagation();
 520+ $form.submit();
 521+ } )
 522+ .button({ disabled: true });
 523+
481524 $examplesButton = $( '<button>' )
 525+ .text( mw.msg( 'apisb-examples' ) )
482526 .click( function ( e ) {
483 - e.preventDefault();
484527 $examplesContent.slideToggle();
485528 } )
 529+ .button()
486530 .hide()
487531 .appendTo( $buttonsContainer );
488 - $examplesContent = $( '#api-sandbox-examples' );
489532
 533+ $( '<button>' )
 534+ .text( mw.msg( 'apisb-clear' ) )
 535+ .click( function ( e ) {
 536+ resetUI();
 537+ } )
 538+ .button()
 539+ .appendTo( $buttonsContainer );
 540+
490541 // init caches
491542 paramInfo = { modules: {}, querymodules: {} };
492543 namespaceOptions = [];
——@@ -503,14 +554,6 @@
504555 }
505556 } );
506557
507 - $( '<button>' )
508 - .text( mw.msg( 'apisb-clear' ) )
509 - .click( function ( e ) {
510 - e.preventDefault();
511 - resetUI();
512 - } )
513 - .insertAfter( $examplesButton );
514 -
515558 // load stuff we need from the beginning
516559 getParamInfo(
517560 {
——@@ -554,19 +597,18 @@
555598 } );
556599
557600
558 - $( '#api-sandbox-form' ).submit( function ( e ) {
559 - var url, params, mustBePosted, data;
 601+ $form.submit( function ( e ) {
 602+ var url, params, mustBePosted;
560603
561604 e.preventDefault();
562605
563 - if ( $submit.prop( 'disabled' ) === true ) {
 606+ if ( $submit.button( 'option', 'disabled' ) === true ) {
564607 return;
565608 }
566609
567610 url = mw.util.wikiScript( 'api' ) + '?' + $.param({ action: $action.val() });
568611 params = mainRequest.getRequestData();
569612 mustBePosted = mainRequest.info.mustbeposted === '';
570 - debugger;
571613 if ( $action.val() === 'query' ) {
572614 url += '&' + $query.val();
573615 params += queryRequest.getRequestData();
——@@ -591,7 +633,7 @@
592634 $postRow.hide();
593635 }
594636 url = url.replace( /(&format=[^&]+)/, '$1fm' );
595 - data = {
 637+ $.ajax({
596638 url: url,
597639 data: params,
598640 dataType: 'text',
——@@ -608,9 +650,14 @@
609651 },
610652 error: function () {
611653 showLoadError( $output, 'apisb-request-error' );
 654+ },
 655+ // either success or error
 656+ complete: function () {
 657+ $pageScroll.animate({ scrollTop: $('#api-sandbox-result').offset().top }, 400, function () {
 658+ window.location.hash = '#api-sandbox-result';
 659+ });
612660 }
613 - };
614 - $.ajax( data );
 661+ });
615662 });
616663
617664 });
Index: trunk/extensions/ApiSandbox/ApiSandbox.i18n.php
——@@ -22,6 +22,10 @@
2323 'apisb-select-action' => 'Select action',
2424 'apisb-select-query' => 'What to query?',
2525 'apisb-select-value' => 'Select value',
 26+ 'apisb-docs-more' => 'read more',
 27+ 'apisb-params-param' => 'Parameter',
 28+ 'apisb-params-input' => 'Input',
 29+ 'apisb-params-desc' => 'Description',
2630 'apisb-loading' => 'Loading...',
2731 'apisb-load-error' => 'Error loading API description',
2832 'apisb-request-error' => 'Error performing API request',
——@@ -128,7 +132,7 @@
129133 'apisb-generator-parameters' => 'Генератор',
130134 );
131135
132 -/** Belarusian (Taraškievica orthography) (‪Беларуская (тарашкевіца)‬)
 136+/** Belarusian (Taraškievica orthography) (Беларуская (тарашкевіца))
133137 * @author EugeneZelenko
134138 * @author Jim-by
135139 * @author Wizardist
——@@ -278,7 +282,7 @@
279283 'apisb-clear' => 'Leeren',
280284 );
281285
282 -/** German (formal address) (‪Deutsch (Sie-Form)‬)
 286+/** German (formal address) (Deutsch (Sie-Form))
283287 * @author Kghbln
284288 */
285289 $messages['de-formal'] = array(
——@@ -867,7 +871,7 @@
868872 'apisb-query-list' => 'Leste',
869873 );
870874
871 -/** Kurdish (Latin script) (‪Kurdî (latînî)‬)
 875+/** Kurdish (Latin script) (Kurdî (latînî))
872876 * @author George Animal
873877 */
874878 $messages['ku-latn'] = array(
——@@ -1051,7 +1055,7 @@
10521056 'apisb-clear' => 'Padamkan',
10531057 );
10541058
1055 -/** Norwegian (bokmål)‬ (‪Norsk (bokmål)‬)
 1059+/** Norwegian (bokmål) (Norsk (bokmål))
10561060 * @author Nghtwlkr
10571061 */
10581062 $messages['nb'] = array(
——@@ -1470,7 +1474,7 @@
14711475 'apisb-clear' => 'Počisti',
14721476 );
14731477
1474 -/** Serbian (Cyrillic script) (‪Српски (ћирилица)‬)
 1478+/** Serbian (Cyrillic script) (Српски (ћирилица))
14751479 * @author Rancher
14761480 */
14771481 $messages['sr-ec'] = array(
——@@ -1676,7 +1680,7 @@
16771681 'apisb-query-list' => 'ליסטעס',
16781682 );
16791683
1680 -/** Simplified Chinese (‪中文(简体)‬)
 1684+/** Simplified Chinese (中文(简体))
16811685 * @author Anakmalaysia
16821686 * @author Hydra
16831687 * @author Hzy980512
——@@ -1715,7 +1719,7 @@
17161720 'apisb-examples' => '示例',
17171721 );
17181722
1719 -/** Traditional Chinese (‪中文(繁體)‬)
 1723+/** Traditional Chinese (中文(繁體))
17201724 * @author Anakmalaysia
17211725 * @author Liangent
17221726 */
Index: trunk/extensions/ApiSandbox/ApiSandbox.php
——@@ -31,14 +31,21 @@
3232 'apisb-load-error',
3333 'apisb-request-error',
3434 'apisb-select-value',
35 - 'apisb-namespaces-error',
 35+ 'apisb-docs-more',
 36+ 'apisb-params-param',
 37+ 'apisb-params-input',
 38+ 'apisb-params-desc',
3639 'apisb-ns-main',
3740 'apisb-example',
3841 'apisb-examples',
3942 'apisb-clear',
 43+ 'apisb-submit',
4044 'parentheses',
4145 ),
42 - 'dependencies' => 'mediawiki.util',
 46+ 'dependencies' => array(
 47+ 'mediawiki.util',
 48+ 'jquery.ui.button',
 49+ )
4350 );
4451
4552 $wgHooks['APIGetDescription'][] = 'efASAPIGetDescription';
Index: trunk/extensions/ApiSandbox/ext.apiSandbox.css
——@@ -1,27 +1,76 @@
 2+/* Buttons */
 3+#api-sandbox-buttons {
 4+ text-align: right;
 5+}
 6+
 7+/* Options */
 8+
29 .api-sandbox-options {
310 width: 100%;
 11+ table-layout: fixed;
412 }
513
6 -.api-sandbox-label {
7 - text-align: right;
8 - width: 12em;
 14+.api-sandbox-options th {
 15+ text-align: left;
 16+}
 17+
 18+.api-sandbox-options td,
 19+.api-sandbox-options th {
920 vertical-align: top;
 21+ width: 13em;
1022 }
1123
12 -.api-sandbox-result-label {
 24+.api-sandbox-options select {
1325 width: 12em;
1426 }
1527
16 -.api-sandbox-value {
17 - width: 18em;
 28+.api-sandbox-options .api-sandbox-docs-col {
 29+ width: auto;
 30+}
 31+
 32+/* Params */
 33+.api-sandbox-params {
 34+ width: 100%;
 35+ table-layout: fixed;
 36+}
 37+
 38+.api-sandbox-params td,
 39+.api-sandbox-params th {
 40+ padding: 5px 7px;
 41+}
 42+
 43+.api-sandbox-params-label {
1844 vertical-align: top;
 45+ width: 12em;
 46+ text-align: right;
 47+}
 48+
 49+.api-sandbox-params-value {
 50+ width: 20em;
 51+ vertical-align: top;
1952 overflow: auto;
2053 }
2154
 55+.api-sandbox-params input[type="text"] {
 56+ padding: 2px 3px;
 57+ width: 19em;
 58+}
 59+
 60+th.api-sandbox-params-label,
 61+th.api-sandbox-params-value {
 62+ text-align: center;
 63+}
 64+
 65+/* Result */
 66+
2267 .api-sandbox-result-container {
2368 width: 100%;
2469 }
2570
 71+.api-sandbox-result-label {
 72+ width: 12em;
 73+}
 74+
2675 #api-sandbox-input {
2776 width: 17em;
2877 }

Sign-offs

UserFlagDate
Reedytested23:56, 2 February 2012

Follow-up revisions

RevisionCommit summaryAuthorDate
r109779[ApiSandbox] fix regression from r109726 in some browsers...krinkle23:19, 22 January 2012
r109785[ApiSandbox] fix regression from r109726 in some browsers...krinkle00:54, 23 January 2012
r109788[ApiSandbox] rm leftover from r109726krinkle01:14, 23 January 2012
r110174follow-up to r109726, removing useless replace functionkaldari21:57, 27 January 2012
r112702[ApiSandbox] Prevent duplicate submission of the form when clicking "Make Req...krinkle18:03, 29 February 2012
r112721[ApiSandbox] Prevent duplicate submission of the form when clicking "Make Req...krinkle20:52, 29 February 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r109721[ApiSandbox] front-end code clean up...krinkle04:58, 22 January 2012

Comments

#Comment by Krinkle (talk | contribs)   11:17, 22 January 2012
#Comment by Reedy (talk | contribs)   14:39, 22 January 2012

Nice

#Comment by Reedy (talk | contribs)   17:11, 22 January 2012

Testing on my laptop (1366 x 768) a lot of the input textboxes end up with scrollbars...

#Comment by Krinkle (talk | contribs)   21:52, 22 January 2012

Done (+ more).

#Comment by Kaldari (talk | contribs)   19:29, 27 January 2012 
+    descTxt = smartEscape( this.info.description );
...
+    descTxt = descTxt.replace( /^([^\r\n\.]*)/, '$1' ) + ' ';

The second line doesn't do anything but add a space (as the functionality of the regex has been removed). You could replace the above lines with just: 

descTxt = smartEscape( this.info.description ) + ' ';
#Comment by Krinkle (talk | contribs)   20:39, 27 January 2012

I had my doubts at the purpose of this regex. And from reading your comment I was right. However since that regex is not related to this commit and I don't feel comfortable playing with that regex, I'm marking this back "new". If you think it should change, feel free to do so. I only moved the existing one introduce by r94449 a line down during clean up and creation of the apisb-docs-more message (making the link on "more" instead of the entire sentence being an anchor).

#Comment by Kaldari (talk | contribs)   21:57, 27 January 2012

That's fine. I removed it in r110174.

#Comment by Hashar (talk | contribs)   10:39, 29 February 2012

Cause bug 34790 : Pressing "Make Request" generates two calls to api.php

#Comment by Hashar (talk | contribs)   20:28, 29 February 2012

Solved by Krinkle with r112702

#Comment by Hashar (talk | contribs)   20:54, 29 February 2012

Really solved by Krinkle with r112721

Status & tagging log