r94959 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r94958‎ | r94959 | r94960 >
Date:23:09, 18 August 2011
Author:salvatoreingala
Status:deferred
Tags:
Comment:
- Removed ugly alerts used by ext.gadgets.preferences.js when saving preferences, using pretty messages instead.
- Now the "Save" button is disabled if there are no changes.
- Minor improvements and documentation in jquery.formbuilder.js.
Modified paths:
  • /branches/salvatoreingala/Gadgets/Gadgets.i18n.php (modified) (history)
  • /branches/salvatoreingala/Gadgets/backend/GadgetHooks.php (modified) (history)
  • /branches/salvatoreingala/Gadgets/ui/resources/ext.gadgets.preferences.css (added) (history)
  • /branches/salvatoreingala/Gadgets/ui/resources/ext.gadgets.preferences.js (modified) (history)
  • /branches/salvatoreingala/Gadgets/ui/resources/jquery.formBuilder.js (modified) (history)

Diff [purge]

Index: branches/salvatoreingala/Gadgets/Gadgets.i18n.php
——@@ -46,8 +46,9 @@
4747 'gadgets-configure' => 'Configure',
4848 'gadgets-configuration-of' => 'Configuration of $1',
4949 'gadgets-prefs-save' => 'Save',
50 - 'gadgets-prefs-cancel' => 'Cancel',
 50+ 'gadgets-prefs-close' => 'Close',
5151 'gadgets-unexpected-error' => 'An unexpected error occurred. Please check your connection. If the problems persists, please contact the developers.',
 52+ 'gadgets-save-invalid' => 'There are fields with invalid value.',
5253 'gadgets-save-success' => 'Settings saved successfully.',
5354 'gadgets-save-failed' => 'Failed to save settings. If the problems persists, please contact the developers.',
5455 'gadgets-ajax-wrongparams' => 'An AJAX request with wrong parameters has been made; this is most likely a bug.',
Index: branches/salvatoreingala/Gadgets/backend/GadgetHooks.php
——@@ -131,14 +131,15 @@
132132 //Register the ext.gadgets.preferences module
133133 //TODO: fix caching issues for user-defined messages
134134 $resourceLoader->register( 'ext.gadgets.preferences', array(
135 - 'scripts' => array( 'ext.gadgets.preferences.js' ),
136 - 'dependencies' => array(
 135+ 'scripts' => array( 'ext.gadgets.preferences.js' ),
 136+ 'styles' => array( 'ext.gadgets.preferences.css' ),
 137+ 'dependencies' => array(
137138 'jquery', 'jquery.json', 'jquery.ui.dialog', 'jquery.formBuilder',
138139 'mediawiki.htmlform', 'ext.gadgets'
139140 ),
140141 'messages' => array_merge( $messages, array(
141 - 'gadgets-configure', 'gadgets-configuration-of', 'gadgets-prefs-save', 'gadgets-prefs-cancel',
142 - 'gadgets-unexpected-error', 'gadgets-save-success', 'gadgets-save-failed'
 142+ 'gadgets-configure', 'gadgets-configuration-of', 'gadgets-prefs-save', 'gadgets-prefs-close',
 143+ 'gadgets-unexpected-error', 'gadgets-save-invalid', 'gadgets-save-success', 'gadgets-save-failed'
143144 ) ),
144145 'localBasePath' => dirname( dirname( __FILE__ ) ) . '/ui/resources/',
145146 'remoteExtPath' => 'Gadgets/ui/resources'
Index: branches/salvatoreingala/Gadgets/ui/resources/jquery.formBuilder.js
——@@ -599,6 +599,20 @@
600600 return new F();
601601 }
602602
 603+ //Add a "smart" listener to watch for changes to an <input /> element
 604+ //This binds to several events, but calls the callback only if the value actually changed
 605+ function addSmartChangeListener( $input, callback ) {
 606+ var oldValue = $input.val();
 607+ //bind all events that may change the value of the field (some are brower-specific)
 608+ $input.bind( 'keyup change propertychange input paste', function() {
 609+ var newValue = $input.val();
 610+ if ( oldValue !== newValue ) {
 611+ oldValue = newValue;
 612+ callback();
 613+ }
 614+ } );
 615+ }
 616+
603617 /* Basic interface for fields */
604618 function Field( desc, options ) {
605619 if ( typeof options.idPrefix == 'undefined' ) {
——@@ -721,6 +735,12 @@
722736 name: this.options.idPrefix + this.desc.name
723737 } );
724738
 739+ if ( options.change ) {
 740+ this.$c.change( function() {
 741+ options.change()
 742+ } );
 743+ }
 744+
725745 var value = options.values && options.values[this.desc.name];
726746 if ( typeof value != 'undefined' ) {
727747 if ( typeof value != 'boolean' ) {
——@@ -764,7 +784,7 @@
765785 id: this.options.idPrefix + this.desc.name,
766786 name: this.options.idPrefix + this.desc.name
767787 } );
768 -
 788+
769789 var value = options.values && options.values[this.desc.name];
770790 if ( typeof value != 'undefined' ) {
771791 if ( typeof value != 'string' ) {
——@@ -774,6 +794,11 @@
775795 this.$text.val( value );
776796 }
777797
 798+ //Add the change event listener
 799+ if ( options.change ) {
 800+ addSmartChangeListener( this.$text, options.change );
 801+ }
 802+
778803 this.$div.append( this.$text );
779804 }
780805
——@@ -849,6 +874,11 @@
850875 this.$text.val( value );
851876 }
852877
 878+ //Add the change event listener
 879+ if ( options.change ) {
 880+ addSmartChangeListener( this.$text, options.change );
 881+ }
 882+
853883 this.$div.append( this.$text );
854884 }
855885
——@@ -928,6 +958,13 @@
929959 $select.val( i ).prop( 'selected', 'selected' );
930960 }
931961
 962+ //Add the change event listener
 963+ if ( options.change ) {
 964+ $select.change( function() {
 965+ options.change();
 966+ } );
 967+ }
 968+
932969 this.$div.append( $select );
933970 }
934971
——@@ -973,17 +1010,17 @@
9741011 var $slider = this.$slider = $( '<div/>' )
9751012 .attr( 'id', this.options.idPrefix + this.desc.name );
9761013
977 - var rangeOptions = {
 1014+ var sliderOptions = {
9781015 min: this.desc.min,
9791016 max: this.desc.max
9801017 };
9811018
9821019 if ( typeof value != 'undefined' ) {
983 - rangeOptions.value = value;
 1020+ sliderOptions.value = value;
9841021 }
9851022
9861023 if ( typeof this.desc.step != 'undefined' ) {
987 - rangeOptions.step = this.desc.step;
 1024+ sliderOptions.step = this.desc.step;
9881025 }
9891026
9901027 //A tooltip to show current value.
——@@ -1017,7 +1054,7 @@
10181055 } );
10191056 }
10201057
1021 - $.extend( rangeOptions, {
 1058+ $.extend( sliderOptions, {
10221059 start: function( event, ui ) {
10231060 sliding = true;
10241061 },
——@@ -1034,10 +1071,15 @@
10351072 }
10361073 }, 300 );
10371074 sliding = false;
 1075+ },
 1076+ change: function( event, ui ) {
 1077+ if ( options.change ) {
 1078+ options.change();
 1079+ }
10381080 }
10391081 } );
10401082
1041 - $slider.slider( rangeOptions );
 1083+ $slider.slider( sliderOptions );
10421084
10431085 var $handle = $slider.find( '.ui-slider-handle' )
10441086 .focus( function( event ) {
——@@ -1074,7 +1116,7 @@
10751117 function DateField( desc, options ){
10761118 SimpleField.call( this, desc, options );
10771119
1078 - this.$text = $( '<input/>' )
 1120+ var $text = this.$text = $( '<input/>' )
10791121 .attr( {
10801122 type: 'text',
10811123 id: this.options.idPrefix + this.desc.name,
——@@ -1083,6 +1125,8 @@
10841126 onSelect: function() {
10851127 //Force validation, so that a previous 'invalid' state is removed
10861128 $( this ).valid();
 1129+ //trigger change event on the textbox
 1130+ $text.trigger( 'change' );
10871131 }
10881132 } );
10891133
——@@ -1098,6 +1142,11 @@
10991143 this.$text.datepicker( 'setDate', date );
11001144 }
11011145
 1146+ //Add the change event listener
 1147+ if ( options.change ) {
 1148+ addSmartChangeListener( this.$text, options.change );
 1149+ }
 1150+
11021151 this.$div.append( this.$text );
11031152 }
11041153
——@@ -1196,6 +1245,11 @@
11971246 } )
11981247 .blur( closeColorPicker );
11991248
 1249+ //Add the change event listener
 1250+ if ( options.change ) {
 1251+ addSmartChangeListener( this.$text, options.change );
 1252+ }
 1253+
12001254 this.$div.append( this.$text );
12011255 }
12021256
——@@ -1929,7 +1983,18 @@
19301984 * Main method; takes the given preferences description object and builds
19311985 * the body of the form with the requested fields.
19321986 *
1933 - * @param {Object} options
 1987+ * @param {Object} options options to set properties of the form and to change its behaviour.
 1988+ * Valid options:
 1989+ * idPrefix: compulsory, a unique prefix for all ids of elements created by formBuilder, to avoid conflicts.
 1990+ * msgPrefix: compulsory, a prefix to be added to all messages referred to by the description.
 1991+ * values: optional, a map of values of preferences; if omitted, default values will be used.
 1992+ * editable: optional, defaults to false; true if the form must be editable via the UI; used by the preferences editor.
 1993+ * staticFields: optional, defaults to false; ignored if editable is not true. If both editable and staticFields are true,
 1994+ * insertion or removal of fields is not allowed, but changing field properties is. Used by the preferences
 1995+ * editor.
 1996+ * change: optional, a function that will be called back if the value of a field changes (it's not strictly warranted that
 1997+ * a field value has actually changed).
 1998+ *
19341999 * @return {Element} the object with the requested form body.
19352000 */
19362001 function buildFormBody( options ) {
——@@ -1946,16 +2011,10 @@
19472012 return null;
19482013 }
19492014
1950 - var section = new SectionField( description, {
1951 - idPrefix: options.idPrefix,
1952 - msgPrefix: options.msgPrefix,
1953 - values: options.values,
1954 - editable: options.editable === true,
1955 - staticFields: options.staticFields === true
1956 - } );
1957 -
 2015+ var section = new SectionField( description, options );
19582016 section.getElement().appendTo( $form );
19592017
 2018+ //Initialize validator
19602019 var validator = $form.validate( section.getValidationSettings() );
19612020
19622021 $form.data( 'formBuilder', {
Index: branches/salvatoreingala/Gadgets/ui/resources/ext.gadgets.preferences.css
——@@ -0,0 +1,17 @@
 2+/*
 3+ * Styles for ext.gadgets.preferences module
 4+ */
 5+
 6+#mw-gadgets-prefsDialog-message {
 7+ float: right;
 8+ padding: 4px;
 9+ margin: 4px 25px 4px 4px;
 10+ -o-box-shadow: 2px 2px 4px 3px #aaaaaa;
 11+ -webkit-box-shadow: 2px 2px 3px 3px #aaaaaa;
 12+ -moz-box-shadow: 2px 2px 3px 3px #aaaaaa;
 13+ box-shadow: 2px 2px 3px 3px #aaaaaa;
 14+ -o-border-radius: 5px;
 15+ -webkit-border-radius: 5px;
 16+ -moz-border-radius: 5px;
 17+ border-radius: 5px;
 18+}
Property changes on: branches/salvatoreingala/Gadgets/ui/resources/ext.gadgets.preferences.css
___________________________________________________________________
Added: svn:eol-style
119 + native
Index: branches/salvatoreingala/Gadgets/ui/resources/ext.gadgets.preferences.js
——@@ -2,11 +2,82 @@
33 * JavaScript tweaks for Special:Preferences
44 */
55 ( function( $, mw ) {
 6+
 7+ //Deep comparison of two objects. It assumes that the two objects
 8+ //have the same keys (recursively).
 9+ function deepEquals( a, b ) {
 10+ if ( a === b ) {
 11+ return true;
 12+ }
 13+
 14+ if ( $.isArray( a ) ) {
 15+ if ( !$.isArray( b ) || a.length != b.length ) {
 16+ return false;
 17+ }
 18+
 19+ for ( var i = 0; i < a.length; i++ ) {
 20+ if ( !deepEquals( a[i], b[i] ) ) {
 21+ return false;
 22+ }
 23+ }
 24+ } else {
 25+ if ( typeof a != 'object' || typeof b != 'object' ) {
 26+ return false;
 27+ }
 28+
 29+ for ( var key in a ) {
 30+ if ( a.hasOwnProperty( key ) ) {
 31+ if ( !deepEquals( a[key], b[key] ) ) {
 32+ return false;
 33+ }
 34+ }
 35+ }
 36+ }
 37+ return true;
 38+ }
639
 40+ //Deletes a stylesheet object
 41+ function removeStylesheet( styleSheet ) {
 42+ var owner =
 43+ styleSheet.ownerNode ?
 44+ styleSheet.ownerNode : //not-IE or IE >= 9
 45+ styleSheet.owningElement //IE < 9
 46+ owner.parentNode.removeChild( owner );
 47+ }
 48+
 49+ //Shows a message in the bottom of the dialog, with fading
 50+ function showMsg( msg ) {
 51+ if ( msg === null ) {
 52+ $( '#mw-gadgets-prefsDialog-message' ).fadeTo( 200, 0 );
 53+ } else {
 54+ $( '#mw-gadgets-prefsDialog-message' )
 55+ .text( msg )
 56+ .fadeTo( 200, 1 );
 57+ }
 58+ }
 59+
760 //"Save" button click handler
861 function saveConfig( $dialog, gadget, config ) {
962 var prefsJson = $.toJSON( config );
1063
 64+ //disable all dialog buttons
 65+ $( '#mw-gadgets-prefsDialog-close, #mw-gadgets-prefsDialog-save' ).button( 'disable' );
 66+
 67+ //Set cursor to "wait" for all elements; save the stylesheet so it can be removed later
 68+ var waitCSS = mw.util.addCSS( '* { cursor: wait !important; }' );
 69+
 70+ //just to avoid code duplication
 71+ function error() {
 72+ //Remove "wait" cursor
 73+ removeStylesheet( waitCSS )
 74+
 75+ //Warn the user
 76+ showMsg( mw.msg( 'gadgets-save-failed' ) );
 77+
 78+ //Enable all dialog buttons
 79+ $( '#mw-gadgets-prefsDialog-close, #mw-gadgets-prefsDialog-save' ).button( 'enable' );
 80+ }
 81+
1182 $.ajax( {
1283 url: mw.config.get( 'wgScriptPath' ) + '/api.php',
1384 type: "POST",
——@@ -20,15 +91,21 @@
2192 dataType: "json",
2293 success: function( response ) {
2394 if ( typeof response.error == 'undefined' ) {
24 - alert( mw.msg( 'gadgets-save-success' ) );
25 - $dialog.dialog( 'close' );
 95+ //Remove "wait" cursor
 96+ removeStylesheet( waitCSS )
 97+
 98+ //Notify success to user
 99+ showMsg( mw.msg( 'gadgets-save-success' ) );
 100+
 101+ //enable cancel button (leaving 'save' disabled)
 102+ $( '#mw-gadgets-prefsDialog-close' ).button( 'enable' );
 103+ //update 'savedConfig'
 104+ $dialog.data( 'savedValues', config );
26105 } else {
27 - alert( mw.msg( 'gadgets-save-failed' ) );
 106+ error()
28107 }
29108 },
30 - error: function( response ) {
31 - alert( mw.msg( 'gadgets-save-failed' ) );
32 - }
 109+ error: error
33110 } );
34111 }
35112
——@@ -66,22 +143,45 @@
67144
68145 //Create and show dialog
69146
70 - var prefsDescription = response.description;
71 - var values = response.values;
 147+ var prefsDescription = response.description,
 148+ values = response.values,
 149+ $dialogBody;
72150
73 - var dialogBody = $( prefsDescription ).formBuilder( {
 151+ var $form = $( prefsDescription ).formBuilder( {
74152 msgPrefix: "Gadget-" + gadget + "-",
75153 idPrefix: "gadget-" + gadget + "-preferences-",
76 - values: values
 154+ values: values,
 155+ change: function() {
 156+ //Hide current message, if any
 157+ showMsg( null );
 158+
 159+ var savedValues = $dialogBody.data( 'savedValues' ),
 160+ currentValues = $form.formBuilder( 'getValues' );
 161+ //TODO: use a better way of comparing values...
 162+ if ( !deepEquals( currentValues, savedValues ) ) {
 163+ $( '#mw-gadgets-prefsDialog-save' ).button( 'enable' );
 164+ } else {
 165+ $( '#mw-gadgets-prefsDialog-save' ).button( 'disable' );
 166+ }
 167+ }
77168 } );
78169
79 - $( dialogBody ).submit( function() {
 170+ $form.submit( function() {
80171 return false; //prevent form submission
81172 } );
82173
83 - $( dialogBody ).attr( 'id', 'mw-gadgets-prefsDialog' );
 174+ $dialogBody = $( '<div/>' )
 175+ .attr( 'id', 'mw-gadgets-prefsDialog' )
 176+ .append( $form )
 177+ .data( 'savedValues', values );
84178
85 - $( dialogBody ).dialog( {
 179+ //Add a div to show messages
 180+ $( '<div>&nbsp;</div>' )
 181+ .attr( 'id', 'mw-gadgets-prefsDialog-message' )
 182+ .css( 'opacity', 0 ) //starts invisible
 183+ .appendTo( $dialogBody );
 184+
 185+ $dialogBody.dialog( {
86186 modal: true,
87187 width: 550,
88188 resizable: false,
——@@ -92,18 +192,22 @@
93193 buttons: [
94194 //TODO: add a "Restore defaults" button
95195 {
 196+ id: 'mw-gadgets-prefsDialog-save',
96197 text: mw.msg( 'gadgets-prefs-save' ),
 198+ disabled: true,
97199 click: function() {
98 - var isValid = $( dialogBody ).formBuilder( 'validate' );
99 -
 200+ var isValid = $form.formBuilder( 'validate' );
100201 if ( isValid ) {
101 - var values = $( dialogBody ).formBuilder( 'getValues' );
102 - saveConfig( $( this ), gadget, values );
 202+ var currentValues = $form.formBuilder( 'getValues' );
 203+ saveConfig( $( this ), gadget, currentValues );
 204+ } else {
 205+ showMsg( mw.msg( 'gadgets-save-invalid' ) );
103206 }
104207 }
105208 },
106209 {
107 - text: mw.msg( 'gadgets-prefs-cancel' ),
 210+ id: 'mw-gadgets-prefsDialog-close',
 211+ text: mw.msg( 'gadgets-prefs-close' ),
108212 click: function() {
109213 $( this ).dialog( "close" );
110214 }

Status & tagging log