Jhernandez has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/326519 )
Change subject: Introduce the settings dialog UI ...................................................................... Introduce the settings dialog UI * Port ext.popups.desktop/ext.popups.settings.js to settingsDialog.js * Blank tests for now. Needs Qunit integration tests. * Transform into a factory function for future testing. * Saving functionality is commented out, will be removed when immplemented in the actions * Add new incomplete action saveSettings * Will perform the saveSettings async tasks and then trigger enabling or disabling popups. * Rename action settingsDialogClosed to hideSettings for consistency Change-Id: I3936d3a4bc476de16d76025139be09f1798796c4 --- M extension.json D resources/ext.popups.desktop/ext.popups.settings.js M resources/ext.popups/actions.js A resources/ext.popups/settingsDialog.js D tests/qunit/ext.popups.settings.test.js A tests/qunit/ext.popups/settingsDialog.test.js 6 files changed, 194 insertions(+), 270 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups refs/changes/19/326519/1 diff --git a/extension.json b/extension.json index e4222d9..bf05854 100644 --- a/extension.json +++ b/extension.json @@ -75,6 +75,7 @@ "resources/ext.popups/changeListeners/eventLogging.js", "resources/ext.popups/changeListeners/previewCount.js", "resources/ext.popups/changeListeners/settings.js", + "resources/ext.popups/settingsDialog.js", "resources/ext.popups/boot.js" ], "templates": { diff --git a/resources/ext.popups.desktop/ext.popups.settings.js b/resources/ext.popups.desktop/ext.popups.settings.js deleted file mode 100644 index 1b46ee0..0000000 --- a/resources/ext.popups.desktop/ext.popups.settings.js +++ /dev/null @@ -1,186 +0,0 @@ -( function ( $, mw ) { - - var currentLinkLogData, - /** - * @class mw.popups.settings - * @singleton - */ - settings = {}; - - /** - * The settings' dialog's section element. - * Defined in settings.open - * @property $element - */ - settings.$element = null; - - /** - * Renders the relevant form and labels in the settings dialog - * - * @method render - */ - settings.render = function () { - var path = mw.config.get( 'wgExtensionAssetsPath' ) + '/Popups/resources/ext.popups.desktop/', - choices = [ - { - id: 'simple', - name: mw.message( 'popups-settings-option-simple' ).text(), - description: mw.message( 'popups-settings-option-simple-description' ).text(), - image: path + 'images/hovercard.svg', - isChecked: true - }, - { - id: 'advanced', - name: mw.message( 'popups-settings-option-advanced' ).text(), - description: mw.message( 'popups-settings-option-advanced-description' ).text(), - image: path + 'images/navpop.svg' - }, - { - id: 'off', - name: mw.message( 'popups-settings-option-off' ).text() - } - ]; - - // Check if NavigationPopups is enabled - /*global pg: false*/ - if ( typeof pg === 'undefined' || pg.fn.disablePopups === undefined ) { - // remove the advanced option - choices.splice( 1, 1 ); - } - - // render the template - settings.$element = mw.template.get( 'ext.popups.desktop', 'settings.mustache' ).render( { - heading: mw.message( 'popups-settings-title' ).text(), - closeLabel: mw.message( 'popups-settings-cancel' ).text(), - saveLabel: mw.message( 'popups-settings-save' ).text(), - helpText: mw.message( 'popups-settings-help' ).text(), - okLabel: mw.message( 'popups-settings-help-ok' ).text(), - descriptionText: mw.message( 'popups-settings-description' ).text(), - choices: choices - } ); - - // setup event bindings - settings.$element.find( '.save' ).click( settings.save ); - settings.$element.find( '.close' ).click( settings.close ); - settings.$element.find( '.okay' ).click( function () { - settings.close(); - settings.reloadPage(); - } ); - - $( 'body' ).append( settings.$element ); - }; - - /** - * Save the setting to the device and close the dialog - * - * @method save - */ - settings.save = function () { - var v = $( 'input[name=mwe-popups-setting]:checked', '#mwe-popups-settings' ).val(); - if ( v === 'simple' ) { - mw.popups.saveEnabledState( true ); - settings.reloadPage(); - settings.close(); - } else { - mw.popups.saveEnabledState( false ); - $( '#mwe-popups-settings-form, #mwe-popups-settings .save' ).hide(); - $( '#mwe-popups-settings-help, #mwe-popups-settings .okay' ).show(); - mw.track( 'ext.popups.event', $.extend( {}, currentLinkLogData, { - action: 'disabled' - } ) ); - } - }; - - /** - * Show the settings element and position it correctly - * - * @method open - * @param {Object} logData data to log - */ - settings.open = function ( logData ) { - var - h = $( window ).height(), - w = $( window ).width(); - - currentLinkLogData = logData; - - $( 'body' ).append( $( '<div>' ).addClass( 'mwe-popups-overlay' ) ); - - if ( !settings.$element ) { - settings.render(); - } - - // FIXME: Should recalc on browser resize - settings.$element - .show() - .css( 'left', ( w - settings.$element.outerWidth( true ) ) / 2 ) - .css( 'top', ( h - settings.$element.outerHeight( true ) ) / 2 ); - - return false; - }; - - /** - * Close the setting dialog and remove the overlay. - * If the close button is clicked on the help dialog - * save the setting and reload the page. - * - * @method close - */ - settings.close = function () { - if ( $( '#mwe-popups-settings-help' ).is( ':visible' ) ) { - settings.reloadPage(); - } else { - $( '.mwe-popups-overlay' ).remove(); - settings.$element.hide(); - } - }; - - /** - * Adds a link to the footer to re-enable hovercards - * - * @method addFooterLink - */ - settings.addFooterLink = function () { - var $setting, $footer; - - if ( mw.popups.enabled ) { - return false; - } - - $setting = $( '<li>' ).append( - $( '<a>' ) - .attr( 'href', '#' ) - .text( mw.message( 'popups-settings-enable' ).text() ) - .click( function ( e ) { - settings.open(); - e.preventDefault(); - } ) - ); - $footer = $( '#footer-places, #f-list' ); - - // From https://en.wikipedia.org/wiki/MediaWiki:Gadget-ReferenceTooltips.js - if ( $footer.length === 0 ) { - $footer = $( '#footer li' ).parent(); - } - $footer.append( $setting ); - }; - - /** - * Wrapper around window.location.reload. Exposed for testing purposes only. - * - * @private - * @ignore - */ - settings.reloadPage = function () { - location.reload(); - }; - - $( function () { - if ( !mw.popups.enabled ) { - settings.addFooterLink(); - } - } ); - - mw.popups.settings = settings; - -} )( jQuery, mediaWiki ); diff --git a/resources/ext.popups/actions.js b/resources/ext.popups/actions.js index 87ecee0..aa71472 100644 --- a/resources/ext.popups/actions.js +++ b/resources/ext.popups/actions.js @@ -255,13 +255,25 @@ * * @return {Object} */ - actions.settingsDialogClosed = function () { + actions.hideSettings = function () { return { type: types.SETTINGS_HIDE }; }; /** + * Represents the user saving their settings. + * + * @return {Object} + */ + actions.saveSettings = function () { + return function ( dispatch ) { + // ... + return dispatch( actions.hideSettings() ); + }; + }; + + /** * Represents the queued event being logged * `mw.popups.changeListeners.eventLogging` change listener. * diff --git a/resources/ext.popups/settingsDialog.js b/resources/ext.popups/settingsDialog.js new file mode 100644 index 0000000..704d38c --- /dev/null +++ b/resources/ext.popups/settingsDialog.js @@ -0,0 +1,154 @@ +( function ( mw, $ ) { + + /** + * Creates a render function that will create the settings dialog and return + * a set of methods to operate on it + */ + mw.popups.createSettingsDialogRenderer = function () { + + /** + * Cached settings dialog + * + * @type {jQuery} + */ + var $dialog; + + /** + * Cached settings overlay + * + * @type {jQuery} + */ + var $overlay; + + /** + * Renders the relevant form and labels in the settings dialog + */ + return function ( boundActions ) { + + if ( !$dialog ) { + $dialog = createSettingsDialog(); + $overlay = $( '<div>' ).addClass( 'mwe-popups-overlay' ); + + // Setup event bindings + $dialog.find( '.save' ).click( boundActions.saveSettings ); + $dialog.find( '.close, .okay' ).click( boundActions.hideSettings ); + } + + return { + /** + * Append the dialog and overlay to a DOM element + * @param {HTMLElement} el + */ + appendTo: function (el) { + $overlay.appendTo( el ); + $dialog.appendTo( el ); + }, + + /** + * Show the settings element and position it correctly + */ + show: function () { + var h = $( window ).height(), + w = $( window ).width(); + + $overlay.show(); + + // FIXME: Should recalc on browser resize + $dialog + .show() + .css( 'left', ( w - $dialog.outerWidth( true ) ) / 2 ) + .css( 'top', ( h - $dialog.outerHeight( true ) ) / 2 ); + }, + + /** + * Hide the settings dialog. + */ + hide: function () { + if ( $dialog.find( '#mwe-popups-settings-help' ).is( ':visible' ) ) { + // TODO: Why is this trying to reload the page? + // reloadPage(); + } else { + $overlay.hide(); + $dialog.hide(); + } + } + }; + }; + }; + + /** + * Create the settings dialog + * + * @return {jQuery} settings dialog + */ + function createSettingsDialog() { + var $el, + path = mw.config.get( 'wgExtensionAssetsPath' ) + '/Popups/resources/ext.popups/images/', + choices = [ + { + id: 'simple', + name: mw.msg( 'popups-settings-option-simple' ), + description: mw.msg( 'popups-settings-option-simple-description' ), + image: path + 'hovercard.svg', + isChecked: true + }, + { + id: 'advanced', + name: mw.msg( 'popups-settings-option-advanced' ), + description: mw.msg( 'popups-settings-option-advanced-description' ), + image: path + 'navpop.svg' + }, + { + id: 'off', + name: mw.msg( 'popups-settings-option-off' ) + } + ]; + + // Check if NavigationPopups is enabled + /*global pg: false*/ + if ( typeof pg === 'undefined' || pg.fn.disablePopups === undefined ) { + // remove the advanced option + choices.splice( 1, 1 ); + } + + // render the template + $el = mw.template.get( 'ext.popups', 'settings.mustache' ).render( { + heading: mw.msg( 'popups-settings-title' ), + closeLabel: mw.msg( 'popups-settings-cancel' ), + saveLabel: mw.msg( 'popups-settings-save' ), + helpText: mw.msg( 'popups-settings-help' ), + okLabel: mw.msg( 'popups-settings-help-ok' ), + descriptionText: mw.msg( 'popups-settings-description' ), + choices: choices + } ); + + return $el; + } + + /** + * Save the setting to the device and close the dialog + * + * @method save + * + function save( boundActions ) { + var v = $( 'input[name=mwe-popups-setting]:checked', '#mwe-popups-settings' ).val(), + userSettings = mw.popups.createUserSettings( mw.storage, mw.user ); + + if ( v === 'simple' ) { + // Avoid a refresh if 'enabled' -> 'enabled' + if ( !userSettings.getIsEnabled() ) { + userSettings.setIsEnabled( true ); + // TODO: Why is this trying to reload the page? + // reloadPage(); + } + boundActions.hideSettings(); + } else { + userSettings.setIsEnabled( false ); + $( '#mwe-popups-settings-form, #mwe-popups-settings .save' ).hide(); + $( '#mwe-popups-settings-help, #mwe-popups-settings .okay' ).show(); + } + } + */ + + +} )( mediaWiki, jQuery ); diff --git a/tests/qunit/ext.popups.settings.test.js b/tests/qunit/ext.popups.settings.test.js deleted file mode 100644 index 8a64847..0000000 --- a/tests/qunit/ext.popups.settings.test.js +++ /dev/null @@ -1,83 +0,0 @@ -// render, renderOption, and addFooterLink are already covered in the browser tests - -( function ( $, mw ) { - QUnit.module( 'ext.popups.settings' ); - - QUnit.test( 'save', function ( assert ) { - var jQueryInit = this.sandbox.stub( jQuery.fn, 'init' ), - radioButtonValue; - - QUnit.expect( 2 ); - - this.sandbox.stub( mw.popups.settings, 'reloadPage' ); - this.sandbox.stub( mw.popups.settings, 'close' ); - jQueryInit.withArgs( 'input[name=mwe-popups-setting]:checked', '#mwe-popups-settings' ) - .returns( { - val: function () { - return radioButtonValue; - } - } ); - jQueryInit.withArgs( '#mwe-popups-settings-form, #mwe-popups-settings .save' ) - .returns( { - hide: $.noop - } ); - jQueryInit.withArgs( '#mwe-popups-settings-help, #mwe-popups-settings .okay' ) - .returns( { - show: $.noop - } ); - - radioButtonValue = 'simple'; - mw.popups.settings.save(); - assert.equal( - mw.storage.get( 'mwe-popups-enabled' ), - '1', - 'Popups are enabled when the `simple` radio button is checked.' - ); - - radioButtonValue = 'off'; - mw.popups.settings.save(); - assert.equal( - mw.storage.get( 'mwe-popups-enabled' ), - '0', - 'Popups are disabled when the `off` radio button is checked.' - ); - - jQueryInit.restore(); - mw.popups.settings.reloadPage.restore(); - mw.popups.settings.close.restore(); - } ); - - QUnit.test( 'open', function ( assert ) { - QUnit.expect( 2 ); - - mw.popups.settings.open(); - assert.equal( - ( $( window ).width() - mw.popups.settings.$element.outerWidth( true ) ) / 2 + 'px', - mw.popups.settings.$element.css( 'left' ), - 'Settings dialog is horizontally aligned in the middle.' - ); - assert.equal( - ( $( window ).height() - mw.popups.settings.$element.outerHeight( true ) ) / 2 + 'px', - mw.popups.settings.$element.css( 'top' ), - 'Settings dialog is vertically aligned in the middle.' - ); - mw.popups.settings.close(); - } ); - - QUnit.test( 'close', function ( assert ) { - QUnit.expect( 2 ); - - mw.popups.settings.open(); - assert.equal( - mw.popups.settings.$element.is( ':visible' ), - true, - 'Settings dialog is visible when settings are opened.' - ); - mw.popups.settings.close(); - assert.equal( - mw.popups.settings.$element.is( ':visible' ), - false, - 'Settings dialog is not visible when settings are closed.' - ); - } ); -} )( jQuery, mediaWiki ); diff --git a/tests/qunit/ext.popups/settingsDialog.test.js b/tests/qunit/ext.popups/settingsDialog.test.js new file mode 100644 index 0000000..e4bed7e --- /dev/null +++ b/tests/qunit/ext.popups/settingsDialog.test.js @@ -0,0 +1,26 @@ +( function ( $, mw ) { + QUnit.module( 'ext.popups/settingsDialog' ); + + QUnit.test( '#render', function ( assert ) { + var boundActions = { + saveSettings: function() {}, + hideSettings: function() {} + }, + expected = { + appendTo: function () {}, + show: function () {}, + hide: function () {} + }, + result = mw.popups.createSettingsDialogRenderer()( boundActions ); + + // Specifically NOT a deep equal. Only structure. + assert.propEqual( + result, + expected + ); + } ); + + // FIXME: Add Qunit integration tests about the rendering and the behavior of + // the settings dialog. + +} )( jQuery, mediaWiki ); -- To view, visit https://gerrit.wikimedia.org/r/326519 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3936d3a4bc476de16d76025139be09f1798796c4 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