jenkins-bot has submitted this change and it was merged.
Change subject: Hygiene: Introduce Toggler class, kill toggle instance
......................................................................
Hygiene: Introduce Toggler class, kill toggle instance
This is going to be needed to be able to emit events on the opening
and collapsing on sections but not in table of contents.
It also allows us to detach toggling in
table of contents from the sections in the long term.
Changes:
* Breaking change for anything using the toggle module (there should be
none outside MobileFrontend)
* Minor code refactor
* expandStoredSections now takes a Toggler instance (breaking change
but a private function so shouldn't be being used anyway)
Change-Id: I38c17eacc452942aea8942328c7b26cff0e50d99
---
M resources/mobile.toggle/toggle.js
M resources/skins.minerva.tablet.scripts/toc.js
M resources/skins.minerva.toggling/init.js
M tests/qunit/mobile.toggle/test_toggle.js
4 files changed, 69 insertions(+), 57 deletions(-)
Approvals:
Bmansurov: Looks good to me, approved
jenkins-bot: Verified
diff --git a/resources/mobile.toggle/toggle.js
b/resources/mobile.toggle/toggle.js
index 04ee721..1ed3abe 100644
--- a/resources/mobile.toggle/toggle.js
+++ b/resources/mobile.toggle/toggle.js
@@ -14,6 +14,18 @@
Icon = M.require( 'mobile.startup/Icon' );
/**
+ * A class for enabling toggling
+ *
+ * @class
+ * @param {jQuery.Object} $container to apply toggling to
+ * @param {String} prefix a prefix to use for the id.
+ * @param {Page} [page] to allow storage of session for future visits
+ */
+ function Toggler( $container, prefix, page ) {
+ this._enable( $container, prefix, page );
+ }
+
+ /**
* Using the settings module looks at what sections were previously
expanded on
* existing page.
*
@@ -67,11 +79,12 @@
/**
* Expand sections that were previously expanded before leaving this
page.
+ * @param {Toggler} toggler
* @param {jQuery.Object} $container
* @param {Page} page
* @ignore
*/
- function expandStoredSections( $container, page ) {
+ function expandStoredSections( toggler, $container, page ) {
var $sectionHeading, $headline,
expandedSections = getExpandedSections( page ),
$headlines = $container.find( '.section-heading span' );
@@ -84,7 +97,7 @@
expandedSections[page.title][$headline.attr(
'id' )] &&
!$sectionHeading.hasClass( 'open-block' )
) {
- toggle( $sectionHeading, page );
+ toggler.toggle.call( toggler, $sectionHeading,
page );
}
} );
}
@@ -118,7 +131,7 @@
* @param {jQuery.Object} $heading A heading belonging to a section
* @ignore
*/
- function toggle( $heading ) {
+ Toggler.prototype.toggle = function ( $heading ) {
var isCollapsed = $heading.is( '.open-block' ),
page = $heading.data( 'page' ),
options, indicator;
@@ -126,7 +139,7 @@
$heading.toggleClass( 'open-block' );
$heading.data( 'indicator' ).remove();
- options = $heading.hasClass( 'open-block' ) ? arrowUpOptions :
arrowDownOptions;
+ options = isCollapsed ? arrowUpOptions : arrowDownOptions;
indicator = new Icon( options ).prependTo( $heading );
$heading.data( 'indicator', indicator );
@@ -140,19 +153,20 @@
if ( !browser.isWideScreen() ) {
storeSectionToggleState( $heading, page );
}
- }
+ };
/**
* Enables toggling via enter and space keys
*
* @ignore
+ * @param {Toggler} toggler instance.
* @param {jQuery.Object} $heading
*/
- function enableKeyboardActions( $heading ) {
+ function enableKeyboardActions( toggler, $heading ) {
$heading.on( 'keypress', function ( ev ) {
if ( ev.which === 13 || ev.which === 32 ) {
// Only handle keypresses on the "Enter" or
"Space" keys
- toggle( $( this ) );
+ toggler.toggle.call( toggler, $( this ) );
}
} ).find( 'a' ).on( 'keypress mouseup', function ( ev ) {
ev.stopPropagation();
@@ -166,7 +180,7 @@
* @param {String} selector A css selector that identifies a single
element
* @param {Object} $container jQuery element to search in
*/
- function reveal( selector, $container ) {
+ Toggler.prototype.reveal = function ( selector, $container ) {
var $target, $heading;
// jQuery will throw for hashes containing certain characters
which can break toggling
@@ -178,28 +192,28 @@
$heading = $target.parents(
'.collapsible-block' ).prev( '.collapsible-heading' );
}
if ( $heading.length && !$heading.hasClass(
'open-block' ) ) {
- toggle( $heading );
+ this.toggle( $heading );
}
if ( $heading.length ) {
// scroll again after opening section (opening
section makes the page longer)
window.scrollTo( 0, $target.offset().top );
}
} catch ( e ) {}
- }
+ };
/**
* Enables section toggling in a given container when
wgMFCollapseSectionsByDefault
* is enabled.
*
- * @method
- * @param {jQuery.object} $container to apply toggling to
+ * @param {jQuery.Object} $container to apply toggling to
* @param {String} prefix a prefix to use for the id.
- * @param {Page} page to allow storage of session for future visits
- * @ignore
+ * @param {Page} [page] to allow storage of session for future visits
+ * @private
*/
- function enable( $container, prefix, page ) {
+ Toggler.prototype._enable = function ( $container, prefix, page ) {
var tagName, expandSections, indicator,
$firstHeading,
+ self = this,
collapseSectionsByDefault = mw.config.get(
'wgMFCollapseSectionsByDefault' );
// Also allow .section-heading if some extensions like Wikibase
@@ -231,7 +245,7 @@
.on( 'click', function ( ev ) {
// prevent taps/clicks on edit
button after toggling (bug 56209)
ev.preventDefault();
- toggle( $( this ) );
+ self.toggle.call( self, $( this
) );
} );
indicator = new Icon( arrowDownOptions
).prependTo( $heading );
$heading.data( 'indicator', indicator );
@@ -247,10 +261,10 @@
'aria-expanded': 'false'
} );
- enableKeyboardActions( $heading );
+ enableKeyboardActions( self, $heading );
if ( browser.isWideScreen() || expandSections )
{
// Expand sections by default on wide
screen devices or if the expand sections setting is set
- toggle( $heading );
+ self.toggle.call( self, $heading );
}
}
@@ -265,7 +279,7 @@
function checkHash() {
var hash = window.location.hash;
if ( hash.indexOf( '#' ) === 0 ) {
- reveal( hash, $container );
+ self.reveal( hash, $container );
}
}
@@ -282,12 +296,12 @@
if ( internalRedirectHash ) {
window.location.hash = internalRedirectHash;
- reveal( internalRedirectHash, $container );
+ self.reveal( internalRedirectHash, $container );
}
}
checkInternalRedirectAndHash();
- checkHash();
+ checkHash( this );
// Restricted to links created by editors and thus outside our
control
$container.find( 'a' ).on( 'click', function () {
// the link might be an internal link with a hash.
@@ -295,23 +309,20 @@
if ( $( this ).attr( 'href' ) !== undefined &&
$( this ).attr( 'href' ).indexOf( '#' ) > -1
) {
- checkHash();
+ checkHash( this );
}
} );
if ( !browser.isWideScreen() ) {
- expandStoredSections( $container, page );
+ expandStoredSections( this, $container, page );
cleanObsoleteStoredSections( page );
}
- }
+ };
- M.define( 'mobile.toggle/toggle', {
- reveal: reveal,
- toggle: toggle,
- enable: enable,
- _getExpandedSections: getExpandedSections,
- _expandStoredSections: expandStoredSections,
- _cleanObsoleteStoredSections: cleanObsoleteStoredSections
- } );
+ Toggler._getExpandedSections = getExpandedSections;
+ Toggler._expandStoredSections = expandStoredSections;
+ Toggler._cleanObsoleteStoredSections = cleanObsoleteStoredSections;
+
+ M.define( 'mobile.toggle/Toggler', Toggler );
}( mw.mobileFrontend, jQuery ) );
diff --git a/resources/skins.minerva.tablet.scripts/toc.js
b/resources/skins.minerva.tablet.scripts/toc.js
index b131989..a959b8c 100644
--- a/resources/skins.minerva.tablet.scripts/toc.js
+++ b/resources/skins.minerva.tablet.scripts/toc.js
@@ -1,6 +1,6 @@
( function ( M, $ ) {
var TableOfContents = M.require( 'mobile.toc/TableOfContents' ),
- toggle = M.require( 'mobile.toggle/toggle' );
+ Toggler = M.require( 'mobile.toggle/Toggler' );
/**
* Create TableOfContents if the given Page has sections and is not the
main page
@@ -10,7 +10,7 @@
* @ignore
*/
function init( page ) {
- var toc,
+ var toc, toggle,
sections = page.getSections(),
$toc = $( '#toc' ),
// TODO: remove wgTOC when caches with old HTML expire
@@ -31,7 +31,7 @@
// otherwise append it to the lead section
toc.appendTo( page.getLeadSectionElement() );
}
- toggle.enable( toc.$el, 'toc-' );
+ toggle = new Toggler( toc.$el, 'toc-' );
}
}
diff --git a/resources/skins.minerva.toggling/init.js
b/resources/skins.minerva.toggling/init.js
index 7450994..e79f230 100644
--- a/resources/skins.minerva.toggling/init.js
+++ b/resources/skins.minerva.toggling/init.js
@@ -1,7 +1,7 @@
( function ( M, $ ) {
var page = M.getCurrentPage(),
$contentContainer = $( '#content #bodyContent' ),
- toggle = M.require( 'mobile.toggle/toggle' );
+ Toggler = M.require( 'mobile.toggle/Toggler' );
/**
* Initialises toggling code.
@@ -15,7 +15,7 @@
function init( $container, prefix, page ) {
// distinguish headings in content from other headings
$container.find( '> h1,> h2,> h3,> h4,> h5,> h6' ).addClass(
'section-heading' );
- toggle.enable( $container, prefix, page );
+ new Toggler( $container, prefix, page );
}
// avoid this running on Watchlist
diff --git a/tests/qunit/mobile.toggle/test_toggle.js
b/tests/qunit/mobile.toggle/test_toggle.js
index 271bfe4..1612751 100644
--- a/tests/qunit/mobile.toggle/test_toggle.js
+++ b/tests/qunit/mobile.toggle/test_toggle.js
@@ -1,10 +1,11 @@
( function ( M, $ ) {
- var sectionHtml = mw.template.get( 'tests.mobilefrontend',
'section.hogan' ).render(),
+ var toggle,
+ sectionHtml = mw.template.get( 'tests.mobilefrontend',
'section.hogan' ).render(),
settings = M.require( 'mobile.settings/settings' ),
browser = M.require( 'mobile.browser/browser' ),
page = { title: 'Toggle test' },
- toggle = M.require( 'mobile.toggle/toggle' );
+ Toggler = M.require( 'mobile.toggle/Toggler' );
/**
* Mobile toggling
@@ -17,7 +18,7 @@
this.$container = $( '<div>' ).html( sectionHtml );
this.$section0 = this.$container.find( 'h2' ).eq( 0 );
this.sandbox.stub( browser, 'isWideScreen' ).returns(
false );
- toggle.enable( this.$container, '', page );
+ toggle = new Toggler( this.$container, '', page );
toggle.toggle( this.$section0 );
},
teardown: function () {
@@ -100,7 +101,7 @@
setup: function () {
this.$container = $( '<div>' ).html( sectionHtml );
this.sandbox.stub( browser, 'isWideScreen' ).returns(
true );
- toggle.enable( this.$container, '', page );
+ toggle = new Toggler( this.$container, '', page );
},
teardown: function () {
window.location.hash = '#';
@@ -124,7 +125,7 @@
this.sandbox.stub( mw.config, 'get' ).withArgs(
'wgMFCollapseSectionsByDefault' ).returns( false );
settings.save( 'expandSections', 'true', true );
this.$container = $( '<div>' ).html( sectionHtml );
- toggle.enable( this.$container, '', page );
+ toggle = new Toggler( this.$container, '', page );
},
teardown: function () {
window.location.hash = '#';
@@ -148,7 +149,7 @@
this.sandbox.stub( mw.config, 'get' ).withArgs(
'wgMFCollapseSectionsByDefault' ).returns( true );
this.$container = $( '<div>' ).html( sectionHtml );
this.sandbox.stub( browser, 'isWideScreen' ).returns(
false );
- toggle.enable( this.$container, '', page );
+ toggle = new Toggler( this.$container, '', page );
},
teardown: function () {
window.location.hash = '#';
@@ -190,11 +191,11 @@
this.sandbox.stub( mw.config, 'get' ).withArgs(
'wgMFCollapseSectionsByDefault' ).returns( true );
this.sandbox.stub( browser, 'isWideScreen' ).returns(
false );
this.$container = $( '<div>' ).html( sectionHtml );
- toggle.enable( this.$container, '', page );
+ toggle = new Toggler( this.$container, '', page );
this.$section = this.$container.find( 'h2' );
this.headline = this.$section.find( 'span' ).attr( 'id'
);
this.pageTitle = page.title;
- this.expandedSections = toggle._getExpandedSections(
page );
+ this.expandedSections = Toggler._getExpandedSections(
page );
},
teardown: function () {
window.location.hash = '#';
@@ -210,14 +211,14 @@
);
toggle.toggle( this.$section );
- this.expandedSections = toggle._getExpandedSections( page );
+ this.expandedSections = Toggler._getExpandedSections( page );
assert.strictEqual( typeof this.expandedSections[
this.pageTitle ][ this.headline ],
'number',
'the just toggled section state has been saved'
);
toggle.toggle( this.$section );
- this.expandedSections = toggle._getExpandedSections( page );
+ this.expandedSections = Toggler._getExpandedSections( page );
assert.strictEqual( this.expandedSections[ this.pageTitle ][
this.headline ],
undefined,
'the just toggled section state has been removed'
@@ -229,14 +230,14 @@
settings.save( 'expandedSections',
JSON.stringify( this.expandedSections )
);
- this.expandedSections = toggle._getExpandedSections( page );
+ this.expandedSections = Toggler._getExpandedSections( page );
assert.strictEqual( typeof this.expandedSections[
this.pageTitle ][ this.headline ],
'number',
'manually created section state has been saved
correctly'
);
- toggle._cleanObsoleteStoredSections( page );
- this.expandedSections = toggle._getExpandedSections( page );
+ Toggler._cleanObsoleteStoredSections( page );
+ this.expandedSections = Toggler._getExpandedSections( page );
assert.strictEqual( this.expandedSections[ this.pageTitle ][
this.headline ],
undefined,
'section, whose store time is manually changed to an
older date,' +
@@ -245,7 +246,7 @@
} );
QUnit.test( 'Expanding already expanded section does not toggle it.',
5, function ( assert ) {
- this.expandedSections = toggle._getExpandedSections( page );
+ this.expandedSections = Toggler._getExpandedSections( page );
assert.strictEqual( $.isEmptyObject( this.expandedSections[
this.pageTitle ] ),
true,
'no expanded sections are stored in localStorage yet'
@@ -266,13 +267,13 @@
'revealed section has open-block class'
);
- this.expandedSections = toggle._getExpandedSections( page );
+ this.expandedSections = Toggler._getExpandedSections( page );
assert.strictEqual( typeof this.expandedSections[
this.pageTitle ][ this.headline ],
'number',
'manually revealed section state has been correctly
saved in localStorage'
);
- toggle._expandStoredSections( this.$container, page );
+ Toggler._expandStoredSections( toggle, this.$container, page );
assert.strictEqual(
this.$section.hasClass( 'open-block' ),
@@ -291,7 +292,7 @@
this.$section = this.$container.find( 'h2' ).eq( 0 );
this.headline = 'First_Section';
this.pageTitle = page.title;
- this.expandedSections = toggle._getExpandedSections(
page );
+ this.expandedSections = Toggler._getExpandedSections(
page );
},
teardown: function () {
window.location.hash = '#';
@@ -311,15 +312,15 @@
// save a toggle state manually
this.expandedSections[ this.pageTitle ][ this.headline ] = (
new Date() ).getTime();
settings.save( 'expandedSections', JSON.stringify(
this.expandedSections ), false );
- this.expandedSections = toggle._getExpandedSections( page );
+ this.expandedSections = Toggler._getExpandedSections( page );
assert.strictEqual( typeof this.expandedSections[
this.pageTitle ][ this.headline ],
'number',
'manually created section state has been saved
correctly'
);
- toggle.enable( this.$container, '', page );
+ toggle = new Toggler( this.$container, '', page );
- this.expandedSections = toggle._getExpandedSections( page );
+ this.expandedSections = Toggler._getExpandedSections( page );
assert.strictEqual( typeof this.expandedSections[
this.pageTitle ][ this.headline ],
'number',
'manually created section state is still active after
toggle.init()'
--
To view, visit https://gerrit.wikimedia.org/r/246432
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I38c17eacc452942aea8942328c7b26cff0e50d99
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <[email protected]>
Gerrit-Reviewer: Bmansurov <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits