Update the settings enabled radio with the enabled state

When opening the settings dialog, the form didn't represent the current
enabled status of the application. This change makes sure that every
time the form is shown, the form state represents the current
application state.

If the application is enabled, then the 'simple' (Enabled) option is
selected on the form. If it is disabled then if there's Navigation
popups, it selects 'advanced' (Advanced popups), if there are not, it
selects 'off' (Disabled).

Changes:
* Change listeners
  * Set the form state every time the form is going to be shown
    * Update tests
* Supporting changes
  * Add settingsDialog#setEnabled which updates the DOM form based on the
    enabled flag and the navpops state
  * Extract isNavPopupsEnabled function in settingsDialog.js to be used in the
    form creation and also when setting the form enabled state
  * Add test verifying changeListeners#settings shows and sets enabled state
    properly when showing the dialog more than one time

Change-Id: Ic660f48d9a78e47c09a192ab86681196d2e01d61

resources/ext.popups/changeListeners/settings.js

@@ -27,6 +27,9 @@
 					settings.appendTo( document.body );
 				}
 
+				// Update the UI settings with the current settings
+				settings.setEnabled( state.preview.enabled );
+
 				settings.show();
 			} else if (
 				prevState.settings.shouldShow === true &&

resources/ext.popups/settingsDialog.js

@@ -81,6 +81,28 @@
 				 */
 				toggleHelp: function ( visible ) {
 					toggleHelp( $dialog, visible );
+				},
+
+				/**
+				 * Update the form depending on the enabled flag
+				 *
+				 * If false and no navpops, then checks 'off'
+				 * If true, then checks 'on'
+				 * If false, and there are navpops, then checks 'advanced'
+				 *
+				 * @param {Boolean} enabled if page previews are enabled
+				 */
+				setEnabled: function ( enabled ) {
+					var name = 'off';
+					if ( enabled ) {
+						name = 'simple';
+					} else if ( isNavPopupsEnabled() ) {
+						name = 'advanced';
+					}
+
+					// Check the appropiate radio button
+					$dialog.find( '#mwe-popups-settings-' + name )
+						.prop( 'checked', true );
 				}
 			};
 		};
@@ -114,9 +136,7 @@
 				}
 			];
 
-		// Check if NavigationPopups is enabled
-		/*global pg: false*/
-		if ( typeof pg === 'undefined' || pg.fn.disablePopups === undefined ) {
+		if ( !isNavPopupsEnabled() ) {
 			// remove the advanced option
 			choices.splice( 1, 1 );
 		}
@@ -164,4 +184,14 @@
 		}
 	}
 
+	/**
+	 * Checks if the NavigationPopups gadget is enabled by looking at the global
+	 * variables
+	 * @returns {Boolean} if navpops was found to be enabled
+	 */
+	function isNavPopupsEnabled() {
+		/*global pg: false*/
+		return typeof pg !== 'undefined' && pg.fn.disablePopups !== undefined;
+	}
+
 } )( mediaWiki, jQuery );

tests/qunit/ext.popups/changeListeners/settings.test.js

@@ -1,4 +1,4 @@
-( function ( mw ) {
+( function ( mw, $ ) {
 
 	QUnit.module( 'ext.popups/changeListeners/settings', {
 		setup: function () {
@@ -7,12 +7,16 @@
 				appendTo: this.sandbox.spy(),
 				show: this.sandbox.spy(),
 				hide: this.sandbox.spy(),
-				toggleHelp: this.sandbox.spy()
+				toggleHelp: this.sandbox.spy(),
+				setEnabled: this.sandbox.spy()
 			};
 			this.render.withArgs( 'actions' ).returns( this.rendered );
 
 			this.defaultState = { settings: { shouldShow: false } };
-			this.showState = { settings: { shouldShow: true } };
+			this.showState = {
+				settings: { shouldShow: true },
+				preview: { enabled: true }
+			};
 			this.showHelpState = {
 				settings: {
 					shouldShow: true,
@@ -49,6 +53,7 @@
 
 		assert.ok( this.render.calledWith( 'actions' ), 'The renderer should be called with the actions' );
 		assert.ok( this.rendered.appendTo.called, 'The rendered object should be in the DOM' );
+		assert.ok( this.rendered.setEnabled.called, 'The rendered form should be up to date' );
 		assert.ok( this.rendered.show.called, 'The rendered object should be showed' );
 	} );
 
@@ -63,6 +68,21 @@
 		assert.notOk( this.rendered.hide.called, 'The rendered object should not be hidden' );
 	} );
 
+	QUnit.test( 'it should show settings and update the form when shouldShow becomes true', function ( assert ) {
+		this.settings( null, this.defaultState );
+		this.settings( this.defaultState, this.showState );
+		this.settings( this.showState, this.defaultState );
+		this.settings( this.defaultState, $.extend( true, {}, this.showState, {
+			preview: { enabled: false }
+		} ) );
+
+		assert.ok( this.render.calledOnce, 'The renderer should be called only the first time' );
+		assert.ok( this.rendered.setEnabled.calledTwice, 'The rendered form should be up to date when shown' );
+		assert.strictEqual( this.rendered.setEnabled.firstCall.args[0], true, 'Set enabled should be called with the current enabled state' );
+		assert.strictEqual( this.rendered.setEnabled.secondCall.args[0], false, 'Set enabled should be called with the current enabled state' );
+		assert.ok( this.rendered.show.calledTwice, 'The rendered object should be showed' );
+	} );
+
 	QUnit.test( 'it should hide settings when shouldShow becomes false', function ( assert ) {
 		this.settings( null, this.defaultState );
 		this.settings( this.defaultState, this.showState );
@@ -89,4 +109,4 @@
 		assert.deepEqual( this.rendered.toggleHelp.secondCall.args, [ false ], 'Help should be hidden' );
 	} );
 
-}( mediaWiki ) );
+}( mediaWiki, jQuery ) );

tests/qunit/ext.popups/settingsDialog.test.js

@@ -10,7 +10,8 @@
 				appendTo: function () {},
 				show: function () {},
 				hide: function () {},
-				toggleHelp: function () {}
+				toggleHelp: function () {},
+				setEnabled: function () {}
 			},
 			result = mw.popups.createSettingsDialogRenderer()( boundActions );