Jhernandez has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/326917 )
Change subject: Wire settings dialog to show on SETTINGS_SHOW action
......................................................................
Wire settings dialog to show on SETTINGS_SHOW action
Now when on a hovercard and clicking the cog, it should show the hovercards
settings menu.
Additional changes:
* Change the changeListener/settings to create and attach the DOM element
just once to the DOM
Change-Id: Id685ddcda9532528fc62b383539788f785089ef0
---
M resources/ext.popups/boot.js
M resources/ext.popups/changeListeners/settings.js
M tests/qunit/ext.popups/changeListeners/settings.test.js
3 files changed, 22 insertions(+), 12 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups
refs/changes/17/326917/1
diff --git a/resources/ext.popups/boot.js b/resources/ext.popups/boot.js
index 4ed9463..5c8ae30 100644
--- a/resources/ext.popups/boot.js
+++ b/resources/ext.popups/boot.js
@@ -31,8 +31,9 @@
* @param {Object} actions
* @param {mw.eventLog.Schema} schema
* @param {ext.popups.UserSettings} userSettings
+ * @param {Function} settingsDialog
*/
- function registerChangeListeners( store, actions, schema, userSettings
) {
+ function registerChangeListeners( store, actions, schema, userSettings,
settingsDialog ) {
// Sugar.
var changeListeners = mw.popups.changeListeners,
@@ -43,6 +44,7 @@
registerChangeListener( store, changeListeners.render( actions
) );
registerChangeListener( store, changeListeners.eventLogging(
actions, schema ) );
registerChangeListener( store, changeListeners.previewCount(
userSettings ) );
+ registerChangeListener( store, changeListeners.settings(
actions, settingsDialog ) );
}
/**
@@ -85,10 +87,12 @@
generateToken = mw.user.generateRandomSessionId,
gateway = createGateway(),
userSettings,
+ settingsDialog,
isUserInCondition,
schema;
userSettings = mw.popups.createUserSettings( mw.storage,
mw.user );
+ settingsDialog = mw.popups.createSettingsDialogRenderer();
isUserInCondition = mw.popups.createExperiment( mw.config,
mw.user, userSettings );
schema = mw.popups.createSchema( mw.config, window );
@@ -104,7 +108,7 @@
) )
);
actions = createBoundActions( store );
- registerChangeListeners( store, actions, schema, userSettings );
+ registerChangeListeners( store, actions, schema, userSettings,
settingsDialog );
actions.boot(
isUserInCondition,
diff --git a/resources/ext.popups/changeListeners/settings.js
b/resources/ext.popups/changeListeners/settings.js
index 589743d..3265f2b 100644
--- a/resources/ext.popups/changeListeners/settings.js
+++ b/resources/ext.popups/changeListeners/settings.js
@@ -9,14 +9,21 @@
*/
mw.popups.changeListeners.settings = function ( boundActions, render ) {
var settings;
+ var shown = false;
return function ( prevState, state ) {
- if ( state.settings.shouldShow && !settings ) {
- settings = render( boundActions );
+ if ( state.settings.shouldShow && !shown) {
+ // Lazily instantiate the settings UI
+ if (!settings) {
+ settings = render( boundActions );
+ settings.appendTo(document.body);
+ }
+
settings.show();
+ shown = true;
} else if ( !state.settings.shouldShow && settings ) {
settings.hide();
- settings = undefined;
+ shown = false;
}
};
};
diff --git a/tests/qunit/ext.popups/changeListeners/settings.test.js
b/tests/qunit/ext.popups/changeListeners/settings.test.js
index 1c06834..2426aea 100644
--- a/tests/qunit/ext.popups/changeListeners/settings.test.js
+++ b/tests/qunit/ext.popups/changeListeners/settings.test.js
@@ -2,11 +2,14 @@
QUnit.module( 'ext.popups/changeListeners/settings', {
setup: function () {
- this.render = this.sandbox.spy();
+ this.render = this.sandbox.stub();
this.rendered = {
+ appendTo: this.sandbox.spy(),
show: this.sandbox.spy(),
hide: this.sandbox.spy()
};
+ this.render.withArgs( 'actions' ).returns(
this.rendered );
+
this.defaultState = { settings: { shouldShow: false } };
this.showState = { settings: { shouldShow: true } };
this.settings =
@@ -27,30 +30,26 @@
} );
QUnit.test( 'it should create settings when shouldShow becomes true',
function ( assert ) {
- this.render.withArgs( 'actions' ).returns( this.rendered );
-
this.settings( null, this.defaultState );
this.settings( this.defaultState, this.showState );
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.show.called, 'The rendered object
should be showed' );
} );
QUnit.test( 'it should not create settings when shouldShow keeps being
true', function ( assert ) {
- this.render.withArgs( 'actions' ).returns( this.rendered );
-
this.settings( null, this.defaultState );
this.settings( this.defaultState, this.showState );
this.settings( this.showState, this.showState );
assert.ok( this.render.calledOnce, 'The renderer should be
called only the first time' );
+ 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 hide settings when shouldShow becomes false',
function ( assert ) {
- this.render.withArgs( 'actions' ).returns( this.rendered );
-
this.settings( null, this.defaultState );
this.settings( this.defaultState, this.showState );
this.settings( this.showState, this.defaultState );
--
To view, visit https://gerrit.wikimedia.org/r/326917
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id685ddcda9532528fc62b383539788f785089ef0
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Jhernandez <jhernan...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
