From 7d32ec80d35be151f4c7061901dc43c0f04b0e63 Mon Sep 17 00:00:00 2001
From: Jan Drewniak <jan.drewniak@gmail.com>
Date: Mon, 7 Feb 2022 14:20:17 -0500
Subject: [PATCH] Collapse ToC by default & expand sections when clicking
 section headings

Collapses sub-sections in the new table of contents by default
(except for non-js and reduced-motion users) and expands the
sections when the top-level section link has been clicked.

Refactors the `activateSection` TableOfContents methods into separate
`activateSection` and `deactivateSection` functions.
Adds `expandSection` and `collapseSection` methods.

Adds triangle icon as a visual expand/collapsed indicator
next to all ToC section headings and are hidden via CSS based on
whether or not the section contains subsections.

Adds test for tableOfContents.

Bug: T299361
Change-Id: I36b3ae7f9f633877683bc17a9444c970d7fa7293
---
 includes/templates/TableOfContents.mustache   |   2 +-
 .../templates/TableOfContents__line.mustache  |   2 +-
 .../TableOfContents__topSection.mustache      |  13 ++
 resources/skins.vector.es6/main.js            |   8 +-
 resources/skins.vector.es6/tableOfContents.js | 206 ++++++++++++++----
 .../components/TableOfContents.less           |  82 +++++--
 skin.json                                     |   3 +-
 tests/jest/tableOfContents.test.js            | 100 +++++++++
 8 files changed, 349 insertions(+), 67 deletions(-)
 create mode 100644 includes/templates/TableOfContents__topSection.mustache
 create mode 100644 tests/jest/tableOfContents.test.js

diff --git a/includes/templates/TableOfContents.mustache b/includes/templates/TableOfContents.mustache
index 0a6b6f708..99a17d8d6 100644
--- a/includes/templates/TableOfContents.mustache
+++ b/includes/templates/TableOfContents.mustache
@@ -4,7 +4,7 @@
 	</div>
 	<ul id="table-of-contents">
 		{{#array-sections}}
-		{{>TableOfContents__line}}
+		{{>TableOfContents__topSection}}
 		{{/array-sections}}
 	</ul>
 </nav>
diff --git a/includes/templates/TableOfContents__line.mustache b/includes/templates/TableOfContents__line.mustache
index 32b180952..7f7b11feb 100644
--- a/includes/templates/TableOfContents__line.mustache
+++ b/includes/templates/TableOfContents__line.mustache
@@ -3,7 +3,7 @@
 		<div class="sidebar-toc-text">
 		<span class="sidebar-toc-numb">{{number}}</span>{{{line}}}</div>
 	</a>
-	<ul>
+	<ul class="sidebar-toc-list">
 		{{#array-sections}}
 		{{>TableOfContents__line}}
 		{{/array-sections}}
diff --git a/includes/templates/TableOfContents__topSection.mustache b/includes/templates/TableOfContents__topSection.mustache
new file mode 100644
index 000000000..b3b3eeae6
--- /dev/null
+++ b/includes/templates/TableOfContents__topSection.mustache
@@ -0,0 +1,13 @@
+<li id="toc-{{anchor}}" class="sidebar-toc-list-item sidebar-toc-level-{{toclevel}}">
+	<a class="sidebar-toc-link" href="#{{anchor}}">
+		<div class="sidebar-toc-text">
+		<span class="sidebar-toc-numb">{{number}}</span>{{{line}}}</div>
+	</a>
+	{{!
+	The following <ul> is placed on *one* line in order to leverage the
+	CSS `:empty` selector and hide the downTriangle icon when there are no sub-sections.
+	(`:empty` means no whitespace).
+	}}
+	<ul class="sidebar-toc-list">{{#array-sections}}{{>TableOfContents__line}}{{/array-sections}}</ul>
+	<span class="mw-ui-icon mw-ui-icon-wikimedia-downTriangle mw-ui-icon-small sidebar-toc-toggle"></span>
+</li>
diff --git a/resources/skins.vector.es6/main.js b/resources/skins.vector.es6/main.js
index bc61bf619..207c429b8 100644
--- a/resources/skins.vector.es6/main.js
+++ b/resources/skins.vector.es6/main.js
@@ -81,10 +81,14 @@ const main = () => {
 
 	const tableOfContents = initTableOfContents( {
 		container: tocElement,
-		onSectionClick: () => {
+		onSectionClick: ( id ) => {
+
 			// eslint-disable-next-line no-use-before-define
 			sectionObserver.pause();
 
+			tableOfContents.expandSection( id );
+			tableOfContents.changeActiveSection( id );
+
 			// T297614: We want the link that the user has clicked inside the TOC to
 			// be "active" (e.g. bolded) regardless of whether the browser's scroll
 			// position corresponds to that section. Therefore, we need to temporarily
@@ -116,7 +120,7 @@ const main = () => {
 			const headline = section.querySelector( HEADLINE_SELECTOR );
 
 			if ( headline ) {
-				tableOfContents.activateSection( TOC_SECTION_ID_PREFIX + headline.id );
+				tableOfContents.changeActiveSection( TOC_SECTION_ID_PREFIX + headline.id );
 			}
 		}
 	} );
diff --git a/resources/skins.vector.es6/tableOfContents.js b/resources/skins.vector.es6/tableOfContents.js
index 6bacf1be7..cc615d923 100644
--- a/resources/skins.vector.es6/tableOfContents.js
+++ b/resources/skins.vector.es6/tableOfContents.js
@@ -1,30 +1,9 @@
 const ACTIVE_SECTION_CLASS = 'sidebar-toc-list-item-active';
+const EXPANDED_SECTION_CLASS = 'sidebar-toc-list-item-expanded';
 const PARENT_SECTION_CLASS = 'sidebar-toc-level-1';
 const LINK_CLASS = 'sidebar-toc-link';
 const LIST_ITEM_CLASS = 'sidebar-toc-list-item';
 
-/**
- * Sets an `ACTIVE_SECTION_CLASS` on the element with an id that matches `id`.
- * If the element is not a top level heading (e.g. element with the
- * `PARENT_SECTION_CLASS`), the top level heading will also have the
- * `ACTIVE_SECTION_CLASS`;
- *
- * @callback ActivateSection
- * @param {string} id The id of the element to be activated in the Table of Contents.
- */
-
-/**
- * Called when a list item is clicked.
- *
- * @callback OnSectionClick
- * @param {string} id The id of the clicked list item.
- */
-
-/**
- * @typedef {Object} TableOfContents
- * @property {ActivateSection} activateSection
- */
-
 /**
  * Initializes the sidebar's Table of Contents.
  *
@@ -38,13 +17,86 @@ module.exports = function tableOfContents( props ) {
 		onSectionClick: () => {}
 	}, props );
 
-	let /** @type {HTMLElement | undefined} */ activeParentSection;
-	let /** @type {HTMLElement | undefined} */ activeChildSection;
+	let /** @type {HTMLElement | undefined} */ activeTopSection;
+	let /** @type {HTMLElement | undefined} */ activeSubSection;
+	let /** @type {Array<HTMLElement>} */ expandedSections = [];
 
 	/**
-	 * @param {string} id
+	 * @typedef {Object} activeSectionIds
+	 * @property {string|undefined} parent - The active  top level section ID
+	 * @property {string|undefined} child - The active subsection ID
+	 */
+
+	/**
+	 * Get the ids of the active sections.
+	 *
+	 * @return {activeSectionIds}
+	 */
+	function getActiveSectionIds() {
+		return {
+			parent: ( activeTopSection ) ? activeTopSection.id : undefined,
+			child: ( activeSubSection ) ? activeSubSection.id : undefined
+		};
+	}
+
+	/**
+	 * Sets an `ACTIVE_SECTION_CLASS` on the element with an id that matches `id`.
+	 * If the element is not a top level heading (e.g. element with the
+	 * `PARENT_SECTION_CLASS`), the top level heading will also have the
+	 * `ACTIVE_SECTION_CLASS`;
+	 *
+	 * @param {string} id The id of the element to be activated in the Table of Contents.
 	 */
 	function activateSection( id ) {
+		const selectedTocSection = document.getElementById( id );
+		const {
+			parent: previousActiveTopId,
+			child: previousActiveSubSectionId
+		} = getActiveSectionIds();
+
+		if (
+			!selectedTocSection ||
+			( previousActiveTopId === id ) ||
+			( previousActiveSubSectionId === id )
+		) {
+			return;
+		}
+
+		const topSection = /** @type {HTMLElement} */ ( selectedTocSection.closest( `.${PARENT_SECTION_CLASS}` ) );
+
+		if ( selectedTocSection === topSection ) {
+			activeTopSection = topSection;
+			activeTopSection.classList.add( ACTIVE_SECTION_CLASS );
+		} else {
+			activeTopSection = topSection;
+			activeSubSection = selectedTocSection;
+			activeTopSection.classList.add( ACTIVE_SECTION_CLASS );
+			activeSubSection.classList.add( ACTIVE_SECTION_CLASS );
+		}
+	}
+
+	/**
+	 * Removes the `ACTIVE_SECTION_CLASS` from all ToC sections.
+	 *
+	 */
+	function deactivateSections() {
+		if ( activeSubSection ) {
+			activeSubSection.classList.remove( ACTIVE_SECTION_CLASS );
+			activeSubSection = undefined;
+		}
+		if ( activeTopSection ) {
+			activeTopSection.classList.remove( ACTIVE_SECTION_CLASS );
+			activeTopSection = undefined;
+		}
+	}
+
+	/**
+	 * Adds the `EXPANDED_SECTION_CLASS` CSS class name
+	 * to a top level heading in the ToC.
+	 *
+	 * @param {string} id
+	 */
+	function expandSection( id ) {
 		const tocSection = document.getElementById( id );
 
 		if ( !tocSection ) {
@@ -53,23 +105,84 @@ module.exports = function tableOfContents( props ) {
 
 		const parentSection = /** @type {HTMLElement} */ ( tocSection.closest( `.${PARENT_SECTION_CLASS}` ) );
 
-		if ( activeChildSection ) {
-			activeChildSection.classList.remove( ACTIVE_SECTION_CLASS );
+		if ( parentSection && expandedSections.indexOf( parentSection ) < 0 ) {
+			parentSection.classList.add( EXPANDED_SECTION_CLASS );
+			expandedSections.push( parentSection );
 		}
-		if ( activeParentSection ) {
-			activeParentSection.classList.remove( ACTIVE_SECTION_CLASS );
-		}
-
-		tocSection.classList.add( ACTIVE_SECTION_CLASS );
-
-		if ( parentSection ) {
-			parentSection.classList.add( ACTIVE_SECTION_CLASS );
-		}
-
-		activeChildSection = tocSection;
-		activeParentSection = parentSection || undefined;
 	}
 
+	/**
+	 * Get the IDs of expanded sections.
+	 *
+	 * @return {Array<string>}
+	 */
+	function getExpandedSectionIds() {
+		return expandedSections.map( ( s ) => s.id );
+	}
+
+	/**
+	 *
+	 * @param {string} id
+	 */
+	function changeActiveSection( id ) {
+
+		const { parent: activeParentId, child: activeChildId } = getActiveSectionIds();
+
+		if ( id === activeParentId && id === activeChildId ) {
+			return;
+		} else {
+			deactivateSections();
+			activateSection( id );
+		}
+	}
+
+	/**
+	 * @param {string} id
+	 * @return {boolean}
+	 */
+	function isTopLevelSection( id ) {
+		const section = document.getElementById( id );
+		return !!section && section.classList.contains( PARENT_SECTION_CLASS );
+	}
+
+	/**
+	 * Removes all `EXPANDED_SECTION_CLASS` CSS class names
+	 * from the top level sections in the ToC.
+	 *
+	 * @param {Array<string>} [selectedIds]
+	 */
+	function collapseSections( selectedIds ) {
+		const sectionIdsToCollapse = selectedIds || getExpandedSectionIds();
+		expandedSections = expandedSections.filter( function ( section ) {
+			if ( sectionIdsToCollapse.indexOf( section.id ) > -1 ) {
+				section.classList.remove( EXPANDED_SECTION_CLASS );
+				return false;
+			}
+			return true;
+		} );
+	}
+
+	/**
+	 * @param {string} id
+	 */
+	function toggleExpandSection( id ) {
+		const expandedSectionIds = getExpandedSectionIds();
+		const indexOfExpandedSectionId = expandedSectionIds.indexOf( id );
+		if ( isTopLevelSection( id ) ) {
+			if ( indexOfExpandedSectionId >= 0 ) {
+				collapseSections( [ id ] );
+			} else {
+				expandSection( id );
+			}
+		}
+	}
+
+	/**
+	 * Called when a list item is clicked.
+	 *
+	 * @callback OnSectionClick
+	 * @param {string} id The id of the clicked list item.
+	 */
 	function bindClickListener() {
 		props.container.addEventListener( 'click', function ( e ) {
 			if (
@@ -82,7 +195,6 @@ module.exports = function tableOfContents( props ) {
 				/** @type {HTMLElement | null} */ ( e.target.closest( `.${LIST_ITEM_CLASS}` ) );
 
 			if ( tocSection && tocSection.id ) {
-				activateSection( tocSection.id );
 				// @ts-ignore
 				props.onSectionClick( tocSection.id );
 			}
@@ -91,7 +203,19 @@ module.exports = function tableOfContents( props ) {
 
 	bindClickListener();
 
+	/**
+	 * @typedef {Object} TableOfContents
+	 * @property {changeActiveSection} changeActiveSection
+	 * @property {expandSection} expandSection
+	 * @property {toggleExpandSection} toggleExpandSection
+	 * @property {string} ACTIVE_SECTION_CLASS
+	 * @property {string} EXPANDED_SECTION_CLASS
+	 */
 	return {
-		activateSection
+		expandSection,
+		changeActiveSection,
+		toggleExpandSection,
+		ACTIVE_SECTION_CLASS,
+		EXPANDED_SECTION_CLASS
 	};
 };
diff --git a/resources/skins.vector.styles/components/TableOfContents.less b/resources/skins.vector.styles/components/TableOfContents.less
index 209db096a..1bbf7b843 100644
--- a/resources/skins.vector.styles/components/TableOfContents.less
+++ b/resources/skins.vector.styles/components/TableOfContents.less
@@ -6,7 +6,7 @@
 	width: 200px;
 	max-height: 90vh;
 	overflow: auto;
-	padding: 12px;
+	padding: 12px 12px 12px 26px;
 	float: left;
 	position: sticky;
 	top: calc( @height-sticky-header + 1.5em );
@@ -26,12 +26,6 @@
 		border: 0;
 	}
 
-	.sidebar-toc-list-item-active {
-		> a {
-			font-weight: bold;
-		}
-	}
-
 	.sidebar-toc-link > * {
 		// Prevent click events on the link's contents so that we can use event
 		// delegation and have the target be the anchor element instead.
@@ -46,27 +40,30 @@
 		padding-bottom: 5px;
 	}
 
-	ul {
+	#table-of-contents,
+	.sidebar-toc-list {
 		margin: 0;
 		list-style: none;
 		line-height: 18px;
+	}
 
-		li {
-			list-style-type: none;
-			padding-left: @subcategory-indent;
+	.sidebar-toc-list-item {
+		display: block;
+		position: relative;
+		list-style-type: none;
+		padding-left: @subcategory-indent;
 
-			a {
-				color: @color-base;
-				font-size: @font-size-base;
-			}
+		a {
+			color: @color-base;
+			font-size: @font-size-base;
+		}
+	}
 
-			&.sidebar-toc-level-1 {
-				padding-left: 0;
+	.sidebar-toc-list-item.sidebar-toc-level-1 {
+		padding-left: 0;
 
-				> a {
-					color: @color-link;
-				}
-			}
+		& > a {
+			color: @color-link;
 		}
 	}
 }
@@ -75,3 +72,46 @@
 .mw-checkbox-hack-checkbox:checked ~ .mw-workspace-container .sidebar-toc {
 	display: none;
 }
+
+// Highlight active section
+.sidebar-toc .sidebar-toc-list-item-active > .sidebar-toc-link {
+	font-weight: bold;
+}
+
+// For no-js users, toggling is disabled and icon is hidden
+.sidebar-toc .sidebar-toc-toggle {
+	display: none;
+}
+
+// Collapse ToC sections by default, excluding no-js or prefers-reduced-motion
+@media ( prefers-reduced-motion: no-preference ) {
+	.client-js .sidebar-toc {
+		.sidebar-toc-level-1 .sidebar-toc-list-item {
+			display: none;
+		}
+
+		.sidebar-toc-level-1.sidebar-toc-list-item-expanded .sidebar-toc-list-item {
+			display: block;
+		}
+
+		.sidebar-toc-toggle {
+			position: absolute;
+			font-size: 0.7em; // reduces size of toggle icon (by an arbitrary amount)
+			top: 4px; // visually center icon (at least at default font size)
+			left: -18px;
+			transform: rotate( -90deg );
+		}
+
+		.sidebar-toc-level-1 > .sidebar-toc-toggle {
+			display: block;
+		}
+
+		.sidebar-toc-level-1 > .sidebar-toc-list:empty + .sidebar-toc-toggle {
+			display: none;
+		}
+
+		.sidebar-toc-level-1.sidebar-toc-list-item-expanded .sidebar-toc-toggle {
+			transform: rotate( 0deg );
+		}
+	}
+}
diff --git a/skin.json b/skin.json
index 9e4786518..4419d0a14 100644
--- a/skin.json
+++ b/skin.json
@@ -258,7 +258,8 @@
 				"logIn",
 				"logOut",
 				"imageGallery",
-				"userGroup"
+				"userGroup",
+				"downTriangle"
 			]
 		},
 		"skins.vector.es6": {
diff --git a/tests/jest/tableOfContents.test.js b/tests/jest/tableOfContents.test.js
new file mode 100644
index 000000000..f606cff57
--- /dev/null
+++ b/tests/jest/tableOfContents.test.js
@@ -0,0 +1,100 @@
+// @ts-nocheck
+const tableOfContents = require( '../../resources/skins.vector.es6/tableOfContents.js' );
+
+const template = `
+    <ul>
+
+        <li id="toc-foo" class="sidebar-toc-level-1">
+            <a href="#foo">foo</a>
+        </li>
+
+        <li id="toc-bar" class="sidebar-toc-level-1">
+            <a href="#bar">bar</a>
+            <ul>
+                <li id="toc-baz">
+                    <a href="#baz">baz</a>
+                </li>
+            </ul>
+        </li>
+
+        <li id="toc-qux" class="sidebar-toc-level-1">
+            <a href="#qux">qux</a>
+        </li>
+
+    </ul>
+`;
+
+let toc, fooSection, barSection, bazSection, quxSection;
+
+beforeEach( () => {
+	document.body.innerHTML = template;
+	toc = tableOfContents( { container: document.body } );
+	fooSection = document.getElementById( 'toc-foo' );
+	barSection = document.getElementById( 'toc-bar' );
+	bazSection = document.getElementById( 'toc-baz' );
+	quxSection = document.getElementById( 'toc-qux' );
+} );
+
+test( 'Table of contents changes the active sections', () => {
+	toc.changeActiveSection( 'toc-foo' );
+	expect(
+		fooSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		!barSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		!bazSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		!quxSection.classList.contains( toc.ACTIVE_SECTION_CLASS )
+	).toEqual( true );
+
+	toc.changeActiveSection( 'toc-bar' );
+	expect(
+		!fooSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		barSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		!bazSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		!quxSection.classList.contains( toc.ACTIVE_SECTION_CLASS )
+	).toEqual( true );
+
+	toc.changeActiveSection( 'toc-baz' );
+	expect(
+		!fooSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		barSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		bazSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		!quxSection.classList.contains( toc.ACTIVE_SECTION_CLASS )
+	).toEqual( true );
+
+	toc.changeActiveSection( 'toc-qux' );
+	expect(
+		!fooSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		!barSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		!bazSection.classList.contains( toc.ACTIVE_SECTION_CLASS ) &&
+		quxSection.classList.contains( toc.ACTIVE_SECTION_CLASS )
+	).toEqual( true );
+} );
+
+test( 'Table of contents expands sections', () => {
+	toc.expandSection( 'toc-foo' );
+	expect(
+		fooSection.classList.contains( toc.EXPANDED_SECTION_CLASS ) &&
+		!barSection.classList.contains( toc.EXPANDED_SECTION_CLASS ) &&
+		!bazSection.classList.contains( toc.EXPANDED_SECTION_CLASS ) &&
+		!quxSection.classList.contains( toc.EXPANDED_SECTION_CLASS )
+	).toEqual( true );
+
+	toc.expandSection( 'toc-bar' );
+	expect(
+		fooSection.classList.contains( toc.EXPANDED_SECTION_CLASS ) &&
+		barSection.classList.contains( toc.EXPANDED_SECTION_CLASS ) &&
+		!bazSection.classList.contains( toc.EXPANDED_SECTION_CLASS ) &&
+		!quxSection.classList.contains( toc.EXPANDED_SECTION_CLASS )
+	).toEqual( true );
+} );
+
+test( 'Table of contents toggles expanded sections', () => {
+	toc.toggleExpandSection( 'toc-foo' );
+	expect(
+		fooSection.classList.contains( toc.EXPANDED_SECTION_CLASS )
+	).toEqual( true );
+
+	toc.toggleExpandSection( 'toc-foo' );
+	expect(
+		fooSection.classList.contains( toc.EXPANDED_SECTION_CLASS )
+	).toEqual( false );
+} );