jenkins-bot has submitted this change and it was merged. Change subject: Add footer link change listener ......................................................................
Add footer link change listener Supporting changes: * Add mw.popups.registerChangeListener, which registers a change listener that will only be called when the state in the store has changed. Change-Id: Ibe6934058327c7f02f7d8092e74a667a5a1c600a --- A doc/change_listener.md M extension.json M resources/ext.popups/actions.js M resources/ext.popups/boot.js A resources/ext.popups/changeListener.js A resources/ext.popups/footerLinkChangeListener.js M resources/ext.popups/index.js A tests/qunit/ext.popups/changeListener.test.js A tests/qunit/ext.popups/footerLinkChangeListener.test.js 9 files changed, 336 insertions(+), 2 deletions(-) Approvals: jenkins-bot: Verified Phuedx: Looks good to me, approved diff --git a/doc/change_listener.md b/doc/change_listener.md new file mode 100644 index 0000000..b991eca --- /dev/null +++ b/doc/change_listener.md @@ -0,0 +1,35 @@ +# Change Listeners + +Redux's [`Store#subscribe`](http://redux.js.org/docs/api/Store.html#subscribe) +allows you to subscribe to updates to the state tree. These updates are +delivered every time an action is dispatched to the store, which may or may not +result in a change of state. + +In the Page Previews codebase, a **change listener** is a function that is only +called when the state tree has changed. As such, change listeners are +predominantly responsible for updating the UI so that it matches the state in +the store. + +## Registering Change Listeners + +**Change listeners** are registered automatically during +[boot](./resources/ext.popups/boot.js) in the `registerChangeListeners` +function. It expects the values of the `mw.popups.changeListeners` map to be +factory functions that accept, currently, the [bound action +creators](http://redux.js.org/docs/api/bindActionCreators.html), i.e. + +```javascript +mw.popups.changeListeners.foo = function ( boundActions ) { + var $link = $( '<a>' ) + .attr( 'href': '#' ) + .click( boundActions.showSettings ); + + return function ( prevState, state ) { + // ... + } +}; +``` + +You'll note that the above **change listener** is effectful and maintains some +local state (`$link`), both of which are acceptable. The former is unavoidable +and the latter is to avoid populating the state tree with unimportant data. diff --git a/extension.json b/extension.json index a7330c9..1562708 100644 --- a/extension.json +++ b/extension.json @@ -64,6 +64,8 @@ "resources/ext.popups/processLinks.js", "resources/ext.popups/gateway.js", "resources/ext.popups/reducers.js", + "resources/ext.popups/changeListener.js", + "resources/ext.popups/footerLinkChangeListener.js", "resources/ext.popups/boot.js" ], "templates": { diff --git a/resources/ext.popups/actions.js b/resources/ext.popups/actions.js index 06aea5a..4aea7a2 100644 --- a/resources/ext.popups/actions.js +++ b/resources/ext.popups/actions.js @@ -86,6 +86,18 @@ }; }; + /** + * Represents the user clicking either the "Enable previews" footer menu link, + * or the "cog" icon that's present on each preview. + * + * @return {Object} + */ + actions.showSettings = function () { + return { + type: 'COG_CLICK' + }; + }; + mw.popups.actions = actions; mw.popups.actionTypes = types; diff --git a/resources/ext.popups/boot.js b/resources/ext.popups/boot.js index 00d770a..902bd72 100644 --- a/resources/ext.popups/boot.js +++ b/resources/ext.popups/boot.js @@ -1,4 +1,4 @@ -( function ( mw, Redux, ReduxThunk ) { +( function ( mw, Redux, ReduxThunk, $ ) { var BLACKLISTED_LINKS = [ '.extiw', '.image', @@ -22,6 +22,23 @@ mw.user, userSettings ); + } + + /** + * Subscribes the registered change listeners to the + * [store](http://redux.js.org/docs/api/Store.html#store). + * + * Change listeners are registered by setting a property on + * `mw.popups.changeListeners`. + * + * @param {Store} store + */ + function registerChangeListeners( store, actions ) { + $.each( mw.popups.changeListeners, function ( _, changeListenerFactory ) { + var changeListener = changeListenerFactory( actions ); + + mw.popups.registerChangeListener( store, changeListener ); + } ); } /** @@ -53,6 +70,7 @@ ) ) ); actions = createBoundActions( store ); + registerChangeListeners( store, actions ); actions.boot( isUserInCondition(), @@ -81,4 +99,4 @@ } ); } ); -}( mediaWiki, Redux, ReduxThunk ) ); +}( mediaWiki, Redux, ReduxThunk, jQuery ) ); diff --git a/resources/ext.popups/changeListener.js b/resources/ext.popups/changeListener.js new file mode 100644 index 0000000..75ee53c --- /dev/null +++ b/resources/ext.popups/changeListener.js @@ -0,0 +1,42 @@ +( function ( mw ) { + + /** + * @typedef {Function} ext.popups.ChangeListener + * @param {Object} prevState The previous state + * @param {Object} state The current state + */ + + /** + * Registers a change listener, which is bound to the + * [store](http://redux.js.org/docs/api/Store.html). + * + * A change listener is a function that is only invoked when the state in the + * [store](http://redux.js.org/docs/api/Store.html) changes. N.B. that there + * may not be a 1:1 correspondence with actions being dispatched to the store + * and the state in the store changing. + * + * See [Store#subscribe](http://redux.js.org/docs/api/Store.html#subscribe) + * for more information about what change listeners may and may not do. + * + * @param {Store} store + * @param {ext.popups.ChangeListener} callback + */ + mw.popups.registerChangeListener = function ( store, callback ) { + // This function is based on the example in [the documentation for + // Store#subscribe](http://redux.js.org/docs/api/Store.html#subscribe), + // which was written by Dan Abramov. + + var state; + + store.subscribe( function () { + var prevState = state; + + state = store.getState(); + + if ( prevState !== state ) { + callback( prevState, state ); + } + } ); + }; + +}( mediaWiki ) ); diff --git a/resources/ext.popups/footerLinkChangeListener.js b/resources/ext.popups/footerLinkChangeListener.js new file mode 100644 index 0000000..81d8a76 --- /dev/null +++ b/resources/ext.popups/footerLinkChangeListener.js @@ -0,0 +1,71 @@ +( function ( mw, $ ) { + + /** + * Creates the link element and appends it to the footer element. + * + * The following elements are considered to be the footer element (highest + * priority to lowest): + * + * # `#footer-places` + * # `#f-list` + * # The parent element of `#footer li`, which is either an `ol` or `ul`. + * + * @return {jQuery} The link element + */ + function createFooterLink() { + var $link = $( '<li>' ).append( + $( '<a>' ) + .attr( 'href', '#' ) + .text( mw.message( 'popups-settings-enable' ).text() ) + ), + $footer; + + // As yet, we don't know whether the link should be visible. + $link.hide(); + + // From https://en.wikipedia.org/wiki/MediaWiki:Gadget-ReferenceTooltips.js, + // which was written by Yair rand <https://en.wikipedia.org/wiki/User:Yair_rand>. + $footer = $( '#footer-places, #f-list' ); + + if ( $footer.length === 0 ) { + $footer = $( '#footer li' ).parent(); + } + + $footer.append( $link ); + + return $link; + } + + /** + * Creates an instance of the footer link change listener. + * + * The change listener covers the following behaviour: + * + * * The "Enable previews" link (the "link") is appended to the footer menu + * (see `createFooterLink` above). + * * When Link Previews are disabled, then the link is shown; otherwise, the + * link is hidden. + * * When the user clicks the link, then the `showSettings` bound action + * creator is called. + * + * @param {Object} boundActions + * @return {et.popups.ChangeListener} + */ + mw.popups.changeListeners.footerLink = function ( boundActions ) { + var $footerLink; + + return function ( prevState, state ) { + if ( $footerLink === undefined ) { + $footerLink = createFooterLink(); + $footerLink.click( boundActions.showSettings ); + } + + if ( state.preview.enabled ) { + $footerLink.hide(); + } else { + $footerLink.show(); + } + }; + }; + +}( mediaWiki, jQuery ) ); diff --git a/resources/ext.popups/index.js b/resources/ext.popups/index.js index 378fd81..9e3a872 100644 --- a/resources/ext.popups/index.js +++ b/resources/ext.popups/index.js @@ -2,4 +2,15 @@ mw.popups = {}; + /** + * Unlike action creators and reducers, change listeners are more complex and + * won't be defined in just one file. Create the `mw.popups.changeListeners` + * namespace here to avoid repeating the following: + * + * ```js + * mw.popups.changeListeners = mw.popups.changeListeners || {}; + * ``` + */ + mw.popups.changeListeners = {}; + }( mediaWiki ) ); diff --git a/tests/qunit/ext.popups/changeListener.test.js b/tests/qunit/ext.popups/changeListener.test.js new file mode 100644 index 0000000..6ffb3de --- /dev/null +++ b/tests/qunit/ext.popups/changeListener.test.js @@ -0,0 +1,56 @@ +( function ( mw ) { + + var stubStore = ( function () { + var state; + + return { + getState: function () { + return state; + }, + setState: function ( value ) { + state = value; + } + }; + }() ); + + QUnit.module( 'ext.popups/changeListener' ); + + QUnit.test( 'it should only call the callback when the state has changed', function ( assert ) { + var spy = this.sandbox.spy(), + boundChangeListener; + + stubStore.subscribe = function ( changeListener ) { + boundChangeListener = changeListener; + }; + + mw.popups.registerChangeListener( stubStore, spy ); + + assert.expect( 4 ); + + stubStore.setState( {} ); + + boundChangeListener(); + boundChangeListener(); + + assert.strictEqual( spy.callCount, 1 ); + assert.ok( spy.calledWith( + undefined, // The initial internal state of the change listener. + {} + ) ); + + stubStore.setState( { + foo: 'bar' + } ); + + boundChangeListener(); + + assert.strictEqual( spy.callCount, 2 ); + assert.ok( spy.calledWith( + {}, + { + foo: 'bar' + } + ) ); + } ); + +}( mediaWiki ) ); diff --git a/tests/qunit/ext.popups/footerLinkChangeListener.test.js b/tests/qunit/ext.popups/footerLinkChangeListener.test.js new file mode 100644 index 0000000..73b7a36 --- /dev/null +++ b/tests/qunit/ext.popups/footerLinkChangeListener.test.js @@ -0,0 +1,87 @@ +( function ( mw, $ ) { + + // Since mw.popups.changeListeners.footerLink manipulates the DOM, this test + // is, by necessity, an integration test. + QUnit.module( 'ext.popups/footerLinkChangeListener @integration', { + setup: function () { + var boundActions = {}, + that = this; + + boundActions.showSettings = that.showSettingsSpy = that.sandbox.spy(); + + that.$footer = $( '<ul>' ) + .attr( 'id', 'footer-places' ) + .appendTo( document.body ); + + that.footerLinkChangeListener = + mw.popups.changeListeners.footerLink( boundActions ); + + that.state = { + preview: { + enabled: false + } + }; + + // A helper method, which should make the following tests more easily + // readable. + that.whenLinkPreviewsBoots = function () { + that.footerLinkChangeListener( undefined, that.state ); + }; + }, + teardown: function () { + this.$footer.remove(); + } + } ); + + QUnit.test( 'it should append the link to the footer menu', function ( assert ) { + var $link; + + assert.expect( 2 ); + + this.whenLinkPreviewsBoots(); + + $link = this.$footer.find( 'li a' ); + + assert.strictEqual( $link.length, 1 ); + assert.ok( + $link.is( ':visible' ), + 'Creating the link and showing/hiding it aren\'t exclusive.' + ); + } ); + + QUnit.test( 'it should show and hide the link', function ( assert ) { + var $link, + prevState; + + assert.expect( 2 ); + + this.whenLinkPreviewsBoots(); + + $link = this.$footer.find( 'li a' ); + + assert.ok( $link.is( ':visible' ) ); + + // --- + + prevState = $.extend( {}, this.state ); + this.state.preview.enabled = true; + + this.footerLinkChangeListener( prevState, this.state ); + + assert.notOk( $link.is( ':visible' ) ); + } ); + + QUnit.test( 'it should call the showSettings bound action creator', function ( assert ) { + var $link; + + assert.expect( 1 ); + + this.whenLinkPreviewsBoots(); + + $link = this.$footer.find( 'li a' ); + $link.click(); + + assert.ok( this.showSettingsSpy.called ); + } ); + +}( mediaWiki, jQuery ) ); -- To view, visit https://gerrit.wikimedia.org/r/321851 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibe6934058327c7f02f7d8092e74a667a5a1c600a Gerrit-PatchSet: 4 Gerrit-Project: mediawiki/extensions/Popups Gerrit-Branch: mpga Gerrit-Owner: Phuedx <[email protected]> Gerrit-Reviewer: Phuedx <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
