Jhernandez has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/327185 )
Change subject: Update the settings enabled radio with the enabled state
......................................................................
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
---
M resources/ext.popups/changeListeners/settings.js
M resources/ext.popups/settingsDialog.js
M tests/qunit/ext.popups/changeListeners/settings.test.js
M tests/qunit/ext.popups/settingsDialog.test.js
4 files changed, 62 insertions(+), 7 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups
refs/changes/85/327185/1
diff --git a/resources/ext.popups/changeListeners/settings.js
b/resources/ext.popups/changeListeners/settings.js
index b136157..6226d49 100644
--- a/resources/ext.popups/changeListeners/settings.js
+++ b/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 &&
diff --git a/resources/ext.popups/settingsDialog.js
b/resources/ext.popups/settingsDialog.js
index e706209..23700fc 100644
--- a/resources/ext.popups/settingsDialog.js
+++ b/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,8 @@
}
];
- // 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 +185,13 @@
}
}
+ /**
+ * Checks if the NavigationPopups gadget is enabled by looking at the
global
+ * variables
+ * @returns {Boolean} if navpops was found to be enabled
+ */
+ function isNavPopupsEnabled() {
+ return typeof pg !== 'undefined' && pg.fn.disablePopups !==
undefined;
+ }
+
} )( mediaWiki, jQuery );
diff --git a/tests/qunit/ext.popups/changeListeners/settings.test.js
b/tests/qunit/ext.popups/changeListeners/settings.test.js
index 4b2d0f2..9e032e8 100644
--- a/tests/qunit/ext.popups/changeListeners/settings.test.js
+++ b/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' );
} );
@@ -61,6 +66,22 @@
assert.ok( this.rendered.appendTo.calledOnce, 'The rendered
object should be in the DOM' );
assert.ok( this.rendered.show.calledOnce, 'The rendered object
should be showed just once' );
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.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 ) {
@@ -89,4 +110,4 @@
assert.deepEqual( this.rendered.toggleHelp.secondCall.args, [
false ], 'Help should be hidden' );
} );
-}( mediaWiki ) );
+}( mediaWiki, jQuery ) );
diff --git a/tests/qunit/ext.popups/settingsDialog.test.js
b/tests/qunit/ext.popups/settingsDialog.test.js
index 21e9ef9..db93a10 100644
--- a/tests/qunit/ext.popups/settingsDialog.test.js
+++ b/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 );
--
To view, visit https://gerrit.wikimedia.org/r/327185
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic660f48d9a78e47c09a192ab86681196d2e01d61
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Jhernandez <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
