Merge "Table of contents: FIXME cleanup"

.eslintrcEs6.json

@@ -22,6 +22,7 @@
 		}
 	},
 	"rules": {
+		"compat/compat": "off",
 		"mediawiki/class-doc": "off"
 	}
 }

resources/skins.vector.es6/main.js

@@ -139,16 +139,17 @@ const updateTocLocation = () => {
 	pinnableElement.movePinnableElement( TOC_ID, newContainerId );
 };
 
-const setupTableOfContents = () => {
-	const header = document.getElementById( stickyHeader.STICKY_HEADER_ID );
-	const tocElement = document.getElementById( TOC_ID );
-	const bodyContent = document.getElementById( BODY_CONTENT_ID );
-
+/**
+ * @param {HTMLElement|null} header
+ * @param {HTMLElement|null} tocElement
+ * @param {HTMLElement|null} bodyContent
+ * @param {initSectionObserver} initSectionObserverFn
+ * @return {tableOfContents|null}
+ */
+const setupTableOfContents = ( header, tocElement, bodyContent, initSectionObserverFn ) => {
 	if ( !(
 		tocElement &&
-		bodyContent &&
-		window.IntersectionObserver &&
-		window.requestAnimationFrame
+		bodyContent
 	) ) {
 		return null;
 	}
@@ -199,7 +200,7 @@ const setupTableOfContents = () => {
 	].map( ( tag ) => `.mw-parser-output ${tag}` ).join( ',' );
 	const elements = () => bodyContent.querySelectorAll( `${headingSelector}, .mw-body-content` );
 
-	const sectionObserver = initSectionObserver( {
+	const sectionObserver = initSectionObserverFn( {
 		elements: elements(),
 		topMargin: header ? header.getBoundingClientRect().height : 0,
 		onIntersection: getHeadingIntersectionHandler( tableOfContents.changeActiveSection )
@@ -211,7 +212,13 @@ const setupTableOfContents = () => {
 	mw.hook( 've.activationStart' ).add( () => {
 		sectionObserver.pause();
 	} );
-	mw.hook( 'wikipage.tableOfContents.vector' ).add( updateElements );
+	// @ts-ignore
+	mw.hook( 'wikipage.tableOfContents' ).add( function ( sections ) {
+		tableOfContents.reloadTableOfContents( sections ).then( function () {
+			mw.hook( 'wikipage.tableOfContents.vector' ).fire( sections );
+			updateElements();
+		} );
+	} );
 	mw.hook( 've.deactivationComplete' ).add( updateElements );
 	return tableOfContents;
 };
@@ -220,6 +227,9 @@ const setupTableOfContents = () => {
  * @return {void}
  */
 const main = () => {
+	const isIntersectionObserverSupported = 'IntersectionObserver' in window;
+	const header = document.getElementById( stickyHeader.STICKY_HEADER_ID );
+
 	limitedWidthToggle();
 	// Initialize the search toggle for the main header only. The sticky header
 	// toggle is initialized after Codex search loads.
@@ -236,13 +246,18 @@ const main = () => {
 	//
 	//  Table of contents
 	//
-	const tableOfContents = setupTableOfContents();
+	const tocElement = document.getElementById( TOC_ID );
+	const bodyContent = document.getElementById( BODY_CONTENT_ID );
+
+	const isToCUpdatingAllowed = isIntersectionObserverSupported &&
+		window.requestAnimationFrame;
+	const tableOfContents = isToCUpdatingAllowed ?
+		setupTableOfContents( header, tocElement, bodyContent, initSectionObserver ) : null;
 
 	//
 	// Sticky header
 	//
 	const
-		header = document.getElementById( stickyHeader.STICKY_HEADER_ID ),
 		stickyIntersection = document.getElementById( stickyHeader.FIRST_HEADING_ID ),
 		userLinksDropdown = document.getElementById( stickyHeader.USER_LINKS_DROPDOWN_ID ),
 		allowedNamespace = stickyHeader.isAllowedNamespace( mw.config.get( 'wgNamespaceNumber' ) ),
@@ -254,7 +269,7 @@ const main = () => {
 		!!userLinksDropdown &&
 		allowedNamespace &&
 		allowedAction &&
-		'IntersectionObserver' in window;
+		isIntersectionObserverSupported;
 
 	const { showStickyHeader, disableEditIcons } = initStickyHeaderABTests(
 		ABTestConfig,
@@ -318,6 +333,7 @@ const main = () => {
 module.exports = {
 	main,
 	test: {
+		setupTableOfContents,
 		initStickyHeaderABTests,
 		getHeadingIntersectionHandler
 	}

resources/skins.vector.es6/sectionObserver.js

@@ -17,6 +17,12 @@
  * handler should be throttled.
  */
 
+/**
+ * @callback initSectionObserver
+ * @param {SectionObserverProps} props
+ * @return {SectionObserver}
+ */
+
 /**
  * Observe intersection changes with the viewport for one or more elements. This
  * is intended to be used with the headings in the content so that the

resources/skins.vector.es6/tableOfContents.js

@@ -33,6 +33,12 @@ const TOC_CONTENTS_ID = 'mw-panel-toc-list';
  * @callback onTogglePinned
  */
 
+/**
+ * @callback tableOfContents
+ * @param {TableOfContentsProps} props
+ * @return {TableOfContents}
+ */
+
 /**
  * @typedef {Object} TableOfContentsProps
  * @property {HTMLElement} container The container element for the table of contents.
@@ -379,15 +385,6 @@ module.exports = function tableOfContents( props ) {
 		// Bind event listeners.
 		bindSubsectionToggleListeners();
 		bindPinnedToggleListeners();
-
-		// @ts-ignore
-		// FIXME: Move to resources/skins.vector.es6/main.js, dangerous to register twice.
-		mw.hook( 'wikipage.tableOfContents' ).add( function ( sections ) {
-			// @ts-ignore
-			reloadTableOfContents( sections ).then( function () {
-				mw.hook( 'wikipage.tableOfContents.vector' ).fire( sections );
-			} );
-		} );
 	}
 
 	/**
@@ -425,14 +422,14 @@ module.exports = function tableOfContents( props ) {
 	 * Reloads the table of contents from saved data
 	 *
 	 * @param {Section[]} sections
-	 * @return {JQuery.Promise<any>|Promise<any>}
+	 * @return {Promise<any>}
 	 */
 	function reloadTableOfContents( sections ) {
 		if ( sections.length < 1 ) {
 			reloadPartialHTML( TOC_CONTENTS_ID, '' );
-			return Promise.resolve();
+			return Promise.resolve( [] );
 		}
-		return mw.loader.using( 'mediawiki.template.mustache' ).then( () => {
+		const load = () => mw.loader.using( 'mediawiki.template.mustache' ).then( () => {
 			const { parent: activeParentId, child: activeChildId } = getActiveSectionIds();
 			reloadPartialHTML( TOC_CONTENTS_ID, getTableOfContentsHTML( sections ) );
 			// Reexpand sections that were expanded before the table of contents was reloaded.
@@ -446,6 +443,11 @@ module.exports = function tableOfContents( props ) {
 				activateSection( activeChildId );
 			}
 		} );
+		return new Promise( ( resolve ) => {
+			load().then( () => {
+				resolve( sections );
+			} );
+		} );
 	}
 
 	/**
@@ -543,6 +545,7 @@ module.exports = function tableOfContents( props ) {
 
 	/**
 	 * @typedef {Object} TableOfContents
+	 * @property {reloadTableOfContents} reloadTableOfContents
 	 * @property {changeActiveSection} changeActiveSection
 	 * @property {expandSection} expandSection
 	 * @property {toggleExpandSection} toggleExpandSection
@@ -554,6 +557,7 @@ module.exports = function tableOfContents( props ) {
 	 * @property {string} TOGGLE_CLASS
 	 */
 	return {
+		reloadTableOfContents,
 		expandSection,
 		changeActiveSection,
 		toggleExpandSection,

tests/jest/__snapshots__/tableOfContents.test.js.snap

@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Table of contents re-rendering re-renders toc when wikipage.tableOfContents hook is fired with empty sections 1`] = `
+exports[`Table of contents reloadTableOfContents re-renders toc when wikipage.tableOfContents hook is fired with empty sections 1`] = `
 "<div id=\\"vector-toc\\" class=\\"vector-toc vector-pinnable-element\\">
 <div class=\\"vector-pinnable-header vector-toc-pinnable-header vector-pinnable-header-pinned\\" data-name=\\"vector-toc\\">
 	<h2 class=\\"vector-pinnable-header-label\\">Contents</h2>
@@ -61,7 +61,7 @@ exports[`Table of contents re-rendering re-renders toc when wikipage.tableOfCont
 "
 `;
 
-exports[`Table of contents re-rendering re-renders toc when wikipage.tableOfContents hook is fired with sections 1`] = `
+exports[`Table of contents reloadTableOfContents re-renders toc when wikipage.tableOfContents hook is fired with sections 1`] = `
 "<div id=\\"vector-toc\\" class=\\"vector-toc vector-pinnable-element\\">
 <div class=\\"vector-pinnable-header vector-toc-pinnable-header vector-pinnable-header-pinned\\" data-name=\\"vector-toc\\">
 	<h2 class=\\"vector-pinnable-header-label\\">Contents</h2>

tests/jest/skins.vector.es6/main.test.js

@@ -153,3 +153,79 @@ describe( 'main.js', () => {
 			} );
 	} );
 } );
+
+const sectionObserverFn = () => ( {
+	pause: () => {},
+	resume: () => {},
+	mount: () => {},
+	unmount: () => {},
+	setElements: () => {}
+} );
+
+describe( 'Table of contents re-rendering', () => {
+	const mockMwHook = () => {
+		/** @type {Object.<string, Function>} */
+		let callback = {};
+		// @ts-ignore
+		jest.spyOn( mw, 'hook' ).mockImplementation( ( name ) => {
+
+			return {
+				add: function ( fn ) {
+					callback[ name ] = fn;
+
+					return this;
+				},
+				fire: ( data ) => {
+					if ( callback[ name ] ) {
+						callback[ name ]( data );
+					}
+				}
+			};
+		} );
+	};
+
+	afterEach( () => {
+		jest.restoreAllMocks();
+	} );
+
+	it( 'listens to wikipage.tableOfContents hook when mounted', () => {
+		mockMwHook();
+		const spy = jest.spyOn( mw, 'hook' );
+		const header = document.createElement( 'header' );
+		const tocElement = document.createElement( 'div' );
+		const bodyContent = document.createElement( 'article' );
+		const toc = test.setupTableOfContents( header, tocElement, bodyContent, sectionObserverFn );
+		expect( toc ).not.toBe( null );
+		expect( spy ).toHaveBeenCalledWith( 'wikipage.tableOfContents' );
+		expect( spy ).not.toHaveBeenCalledWith( 'wikipage.tableOfContents.vector' );
+	} );
+
+	it( 'Firing wikipage.tableOfContents triggers reloadTableOfContents', async () => {
+		mockMwHook();
+		const header = document.createElement( 'header' );
+		const tocElement = document.createElement( 'div' );
+		const bodyContent = document.createElement( 'article' );
+		const toc = test.setupTableOfContents( header, tocElement, bodyContent, sectionObserverFn );
+		if ( !toc ) {
+			// something went wrong
+			expect( true ).toBe( false );
+			return;
+		}
+		const spy = jest.spyOn( toc, 'reloadTableOfContents' ).mockImplementation( () => Promise.resolve() );
+
+		mw.hook( 'wikipage.tableOfContents' ).fire( [
+			// Add new section to see how the re-render performs.
+			{
+				toclevel: 1,
+				number: '4',
+				line: 'bat',
+				anchor: 'bat',
+				'is-top-level-section': true,
+				'is-parent-section': false,
+				'array-sections': null
+			}
+		] );
+
+		expect( spy ).toHaveBeenCalled();
+	} );
+} );

tests/jest/tableOfContents.test.js

@@ -206,12 +206,10 @@ describe( 'Table of contents', () => {
 
 	describe( 'applies the correct aria attributes', () => {
 		test( 'when initialized', () => {
-			const spy = jest.spyOn( mw, 'hook' );
 			const toc = mount();
 			const toggleButton = /** @type {HTMLElement} */ ( barSection.querySelector( `.${toc.TOGGLE_CLASS}` ) );
 
 			expect( toggleButton.getAttribute( 'aria-expanded' ) ).toEqual( 'true' );
-			expect( spy ).toBeCalledWith( 'wikipage.tableOfContents' );
 		} );
 
 		test( 'when expanding sections', () => {
@@ -234,49 +232,15 @@ describe( 'Table of contents', () => {
 		} );
 	} );
 
-	describe( 're-rendering', () => {
-		const mockMwHook = () => {
-			/** @type {Function} */
-			let callback;
-			// @ts-ignore
-			jest.spyOn( mw, 'hook' ).mockImplementation( () => {
-
-				return {
-					add: function ( fn ) {
-						callback = fn;
-
-						return this;
-					},
-					fire: ( data ) => {
-						if ( callback ) {
-							callback( data );
-						}
-					}
-				};
-			} );
-		};
-
-		afterEach( () => {
-			jest.restoreAllMocks();
-		} );
-
-		test( 'listens to wikipage.tableOfContents hook when mounted', () => {
-			const spy = jest.spyOn( mw, 'hook' );
-			mount();
-			expect( spy ).toHaveBeenCalledWith( 'wikipage.tableOfContents' );
-		} );
-
-		test( 're-renders toc when wikipage.tableOfContents hook is fired with empty sections', () => {
-			mockMwHook();
-			mount();
-
-			mw.hook( 'wikipage.tableOfContents' ).fire( [] );
+	describe( 'reloadTableOfContents', () => {
+		test( 're-renders toc when wikipage.tableOfContents hook is fired with empty sections', async () => {
+			const toc = mount();
+			await toc.reloadTableOfContents( [] );
 
 			expect( document.body.innerHTML ).toMatchSnapshot();
 		} );
 
 		test( 're-renders toc when wikipage.tableOfContents hook is fired with sections', async () => {
-			mockMwHook();
 			// @ts-ignore
 			// eslint-disable-next-line compat/compat
 			jest.spyOn( mw.loader, 'using' ).mockImplementation( () => Promise.resolve() );
@@ -326,7 +290,8 @@ describe( 'Table of contents', () => {
 
 			// wikipage.tableOfContents includes the nested sections in the top level
 			// of the array.
-			mw.hook( 'wikipage.tableOfContents' ).fire( [
+
+			await toc.reloadTableOfContents( [
 				// foo
 				SECTIONS[ 0 ],
 				// bar
@@ -351,11 +316,6 @@ describe( 'Table of contents', () => {
 				}
 			] );
 
-			// Wait until the mw.loader.using promise has resolved so that we can
-			// check the DOM after it has been updated.
-			// eslint-disable-next-line compat/compat
-			await Promise.resolve();
-
 			const newToggleButton = /** @type {HTMLElement} */ ( document.querySelector( `#toc-bar .${toc.TOGGLE_CLASS}` ) );
 			expect( newToggleButton ).not.toBeNull();
 			// Check that the sections render in their expanded form.