r94100 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r94099‎ | r94100 | r94101 >
Date:08:08, 9 August 2011
Author:devayon
Status:deferred (Comments)
Tags:
Comment:
improved: hiding format controls, minor JS cleanup
Modified paths:
  • /trunk/extensions/SemanticMediaWiki/specials/AskSpecial/SMW_QueryUIHelper.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/specials/AskSpecial/SMW_SpecialQueryCreator.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SemanticMediaWiki/specials/AskSpecial/SMW_QueryUIHelper.php
@@ -883,10 +883,11 @@
884884 *
885885 * @param string $format
886886 * @param array $paramValues The current values for the parameters (name => value)
 887+ * @param array $ignoredattribs Attributes which should not be generated by this method.
887888 *
888889 * @return string
889890 */
890 - protected function showFormatOptions( $format, array $paramValues ) {
 891+ protected function showFormatOptions( $format, array $paramValues, $ignoredattribs = array() ) {
891892 $printer = SMWQueryProcessor::getResultPrinter( $format, SMWQueryProcessor::SPECIAL_PAGE );
892893
893894 $params = method_exists( $printer, 'getValidatorParameters' ) ? $printer->getValidatorParameters() : array();
@@ -894,8 +895,8 @@
895896 $optionsHtml = array();
896897
897898 foreach ( $params as $param ) {
898 - // Ignore the format parameter, as we got a special control in the GUI for it already.
899 - if ( $param->getName() == 'format' ) {
 899+ // Ignore the parameters for which we have a special control in the GUI already.
 900+ if ( in_array( $param->getName(), $ignoredattribs ) ) {
900901 continue;
901902 }
902903
@@ -907,7 +908,7 @@
908909 array(
909910 'style' => 'width: 30%; padding: 5px; float: left;'
910911 ),
911 - htmlspecialchars( $param->getName() ) . ': ' .
 912+ '<label for="p[' . htmlspecialchars( $param->getName() ) . ']">' . htmlspecialchars( $param->getName() ) . ': </label>' .
912913 $this->showFormatOption( $param, $currentValue ) .
913914 '<br />' .
914915 Html::element( 'em', array(), $param->getDescription() )
@@ -923,7 +924,8 @@
924925 $i = 0;
925926 $rowHtml = '';
926927 $resultHtml = '';
927 -
 928+ $flip_style = true;
 929+ $flip_count = 0;
928930 while ( $option = array_shift( $optionsHtml ) ) {
929931 $rowHtml .= $option;
930932 $i++;
@@ -931,12 +933,18 @@
932934 $resultHtml .= Html::rawElement(
933935 'div',
934936 array(
935 - 'style' => 'background: ' . ( $i % 6 == 0 ? 'white' : '#dddddd' ) . ';'
 937+ 'style' => 'background: ' . ( $flip_style ? 'white' : '#dddddd' ) . ';'
936938 ),
937939 $rowHtml
938940 );
939941
940942 $rowHtml = '';
 943+ $flip_count++;
 944+ if ( $flip_count == 3 ) {
 945+ $flip_style = !$flip_style;
 946+ $flip_count = 0;
 947+ }
 948+
941949 }
942950
943951 return $resultHtml;
Index: trunk/extensions/SemanticMediaWiki/specials/AskSpecial/SMW_SpecialQueryCreator.php
@@ -50,6 +50,21 @@
5151 }
5252
5353 /**
 54+ * Displays a form section showing the options for a given format,
 55+ * based on the getParameters() value for that format's query printer.
 56+ *
 57+ * @param string $format
 58+ * @param array $paramValues The current values for the parameters (name => value)
 59+ * @param array $ignoredattribs Attributes which should not be generated by this method.
 60+ *
 61+ * @return string
 62+ *
 63+ * Overridden from parent to ignore GUI parameters 'format' 'limit' and 'offset'
 64+ */
 65+ protected function showFormatOptions( $format, array $paramValues, $ignoredattribs = array() ) {
 66+ return parent::showFormatOptions( $format, $paramValues, array( 'format', 'limit', 'offset' ) );
 67+ }
 68+ /**
5469 * Creates the input form
5570 *
5671 * @global OutputPage $wgOut
@@ -143,7 +158,9 @@
144159 if ( is_array( $display_values ) ) {
145160 foreach ( $display_values as $key => $value ) {
146161 if ( $value == '1' and array_key_exists( $key, $property_values ) ) {
147 - $po[] = trim( $property_values[$key] );
 162+ $property_values[$key] = trim( $property_values[$key] );
 163+ $property_values[$key] = ( $property_values[$key][0] == '?' ) ? $property_values[$key]:'?' . $property_values[$key];
 164+ $po[] = $property_values[$key];
148165 }
149166
150167 }
@@ -297,7 +314,35 @@
298315 $javascript_text = <<<EOT
299316 <script type="text/javascript">
300317 var num_elements = {$num_sort_values};
301 -// code for handling adding and removing the "sort" inputs
 318+EOT;
 319+// add autocomplete
 320+ if ( $enableAutocomplete == SMWQueryUI::ENABLE_AUTO_SUGGEST ) {
 321+ $javascript_text .= <<<EOT
 322+
 323+function smw_property_autocomplete(){
 324+ jQuery('[name*="property"]').autocomplete({
 325+ minLength: 2,
 326+ source: function(request, response) {
 327+ url=wgScriptPath+'/api.php?action=opensearch&limit=10&namespace='+wgNamespaceIds['property']+'&format=jsonfm';
 328+
 329+ jQuery.getJSON(url, 'search='+request.term, function(data){
 330+ //remove the namespace prefix 'Property:' from returned data
 331+ for(i=0;i<data[1].length;i++) data[1][i]='?'+data[1][i].substr(data[1][i].indexOf(':')+1);
 332+ response(data[1]);
 333+ });
 334+
 335+ }
 336+ });
 337+}
 338+EOT;
 339+ } else {
 340+ $javascript_text .= <<<EOT
 341+function smw_property_autocomplete(){
 342+}
 343+EOT;
 344+ }
 345+
 346+ $javascript_text .= <<<EOT
302347 function smw_prop_code_update(){
303348 code = '?'+\$j('#d-property')[0].value;
304349 if(code!=''){
@@ -334,6 +379,7 @@
335380 \$j('#dialog').dialog.id=prop_id;
336381 \$j('#dialog').dialog('open');
337382 }
 383+// code for handling adding and removing the "sort" inputs
338384 function addPOInstance(starter_div_id, main_div_id) {
339385 var starter_div = document.getElementById(starter_div_id);
340386 var main_div = document.getElementById(main_div_id);
@@ -367,6 +413,7 @@
368414 st='sort_div_'+num_elements;
369415 jQuery('#'+new_div.id).find(".smw-remove a")[0].href="javascript:removePOInstance('"+st+"')";
370416 num_elements++;
 417+ smw_property_autocomplete();
371418 }
372419
373420 function removePOInstance(div_id) {
@@ -396,29 +443,6 @@
397444 }
398445 }
399446 });
400 -});
401 -
402 -jQuery(document).ready(function(){
403 -EOT;
404 - if ( $enableAutocomplete == SMWQueryUI::ENABLE_AUTO_SUGGEST ) {
405 - $javascript_text .= <<<EOT
406 - //add autocomplete
407 - jQuery('[name*="property"]').autocomplete({
408 - minLength: 2,
409 - source: function(request, response) {
410 - url=wgScriptPath+'/api.php?action=opensearch&limit=10&namespace='+wgNamespaceIds['property']+'&format=jsonfm';
411 -
412 - jQuery.getJSON(url, 'search='+request.term, function(data){
413 - //remove the namespace prefix 'Property:' from returned data
414 - for(i=0;i<data[1].length;i++) data[1][i]='?'+data[1][i].substr(data[1][i].indexOf(':')+1);
415 - response(data[1]);
416 - });
417 -
418 - }
419 - });
420 -EOT;
421 - }
422 - $javascript_text .= <<<EOT
423447 jQuery('#sort-more').click(function(){jQuery('#dialog').dialog("open");});
424448 jQuery('#dialog input').bind('keyup click', smw_prop_code_update );
425449
@@ -428,6 +452,7 @@
429453 });
430454 });
431455
 456+jQuery(document).ready(smw_property_autocomplete);
432457 </script>
433458
434459 EOT;

Follow-up revisions

RevisionCommit summaryAuthorDate
r94127style fix (variable names), follow-up: r94100devayon17:07, 9 August 2011

Comments

#Comment by Jeroen De Dauw (talk | contribs)   12:03, 9 August 2011

Some style remarks:

  • It's convention to use lowerCamelCase for variable names. So $ignoredAttribs instead of $ignoredattribs and $flipStyle instead of $flip_style.
  • "$ignoredattribs = array()" should always be an array, so it's nice to specify this in the function with "array $ignoredattribs = array()".

Nice work though! :)

#Comment by Devayon (talk | contribs)   16:19, 9 August 2011

A quick question about style: should local variables camelCase, as documented in MW's manual? or should they be written_like_this, as documented in http://semantic-mediawiki.org/wiki/Programmer%27s_guide_to_SMW#Naming_conventions?

Thanks for pointing that one out! :)

#Comment by Jeroen De Dauw (talk | contribs)   17:22, 9 August 2011

camelCase, not_like_this. If the SMW docs say otherwise, they should be corrected.