Clean up the interface after discarding a reply

Bug: T245574
Change-Id: I016a7a5c44e0d15a153143177976cceb8d6d3d1b

modules/controller.js

@@ -44,8 +44,6 @@ function setupComment( comment ) {
 			$pageContainer.addClass( 'dt-init-replylink-open' );
 
 			if ( !widgetPromise ) {
-				newListItem = modifier.addListItem( comment );
-				$( newListItem ).text( mw.msg( 'discussiontools-replywidget-loading' ) );
 				widgetPromise = replyWidgetPromise.then( function () {
 					var
 						ReplyWidget = config.useVisualEditor ?
@@ -58,10 +56,10 @@ function setupComment( comment ) {
 					replyWidget.on( 'teardown', function () {
 						$link.removeClass( 'dt-init-replylink-active' );
 						$pageContainer.removeClass( 'dt-init-replylink-open' );
-						$( newListItem ).hide();
+						modifier.removeListItem( newListItem );
+						newListItem = null;
 					} );
 
-					$( newListItem ).empty().append( replyWidget.$element );
 					return replyWidget;
 				}, function () {
 					$link.removeClass( 'dt-init-replylink-active' );
@@ -72,9 +70,18 @@ function setupComment( comment ) {
 						type: 'preinit'
 					} );
 				} );
+
+				// On first load, add a placeholder list item
+				newListItem = modifier.addListItem( comment );
+				$( newListItem ).text( mw.msg( 'discussiontools-replywidget-loading' ) );
 			}
+
 			widgetPromise.then( function ( replyWidget ) {
-				$( newListItem ).show();
+				if ( !newListItem ) {
+					// On subsequent loads, there's no list item yet, so create one now
+					newListItem = modifier.addListItem( comment );
+				}
+				$( newListItem ).empty().append( replyWidget.$element );
 				replyWidget.setup();
 				replyWidget.scrollElementIntoView( { padding: scrollPadding } );
 				replyWidget.focus();

modules/modifier.js

@@ -103,7 +103,9 @@ function addListItem( comment ) {
 		// Insert required number of wrappers
 		while ( currLevel < desiredLevel ) {
 			list = target.ownerDocument.createElement( listType );
+			list.discussionToolsModified = 'new';
 			item = target.ownerDocument.createElement( itemType );
+			item.discussionToolsModified = 'new';
 			whitespaceParsoidHack( item );
 
 			parent.insertBefore( list, target.nextSibling );
@@ -121,6 +123,7 @@ function addListItem( comment ) {
 			if ( target.nextSibling ) {
 				// Create new identical node after the parent
 				newNode = parent.cloneNode( false );
+				parent.discussionToolsModified = 'split';
 				parent.parentNode.insertBefore( newNode, parent.nextSibling );
 
 				// Move nodes following target to the new node
@@ -140,6 +143,7 @@ function addListItem( comment ) {
 
 		// parent is now a list, target is a list item
 		item = target.ownerDocument.createElement( target.tagName );
+		item.discussionToolsModified = 'new';
 		whitespaceParsoidHack( item );
 		parent.insertBefore( item, target.nextSibling );
 	}
@@ -147,6 +151,45 @@ function addListItem( comment ) {
 	return item;
 }
 
+/**
+ * Undo the effects of #addListItem, also removing or merging any affected parent nodes.
+ *
+ * @param {HTMLElement} node
+ */
+function removeListItem( node ) {
+	var nextNode;
+
+	while ( node && node.discussionToolsModified ) {
+		if ( node.discussionToolsModified === 'new' ) {
+			nextNode = node.previousSibling || node.parentNode;
+
+			// Remove this node
+			delete node.discussionToolsModified;
+			node.parentNode.removeChild( node );
+
+		} else if ( node.discussionToolsModified === 'split' ) {
+			// Children might be split too, if so, descend into them afterwards
+			if ( node.lastChild && node.lastChild.discussionToolsModified === 'split' ) {
+				node.discussionToolsModified = 'done';
+				nextNode = node.lastChild;
+			} else {
+				delete node.discussionToolsModified;
+				nextNode = node.parentNode;
+			}
+			// Merge the following sibling node back into this one
+			while ( node.nextSibling.firstChild ) {
+				node.appendChild( node.nextSibling.firstChild );
+			}
+			node.parentNode.removeChild( node.nextSibling );
+
+		} else {
+			nextNode = node.parentNode;
+		}
+
+		node = nextNode;
+	}
+}
+
 /**
  * Add another list item after the given one.
  *
@@ -172,6 +215,7 @@ function createWikitextNode( wt ) {
 module.exports = {
 	closest: closest,
 	addListItem: addListItem,
+	removeListItem: removeListItem,
 	addSiblingListItem: addSiblingListItem,
 	createWikitextNode: createWikitextNode
 };

tests/qunit/modifier.test.js

@@ -5,8 +5,10 @@ var
 
 QUnit.module( 'mw.dt.modifier', utils.newEnvironment() );
 
-QUnit.test( '#addListItem', function ( assert ) {
-	var i, j, cases, actualHtml, expectedHtml, comments, node, fixture;
+QUnit.test( '#addListItem/#removeListItem', function ( assert ) {
+	var i, j, cases,
+		actualHtml, expectedHtml, reverseActualHtml, reverseExpectedHtml,
+		comments, nodes, node, fixture;
 
 	cases = [
 		{
@@ -63,18 +65,22 @@ QUnit.test( '#addListItem', function ( assert ) {
 		expectedHtml = fixture.innerHTML;
 
 		$( fixture ).empty().append( cases[ i ].dom.clone() );
+		reverseExpectedHtml = fixture.innerHTML;
+
 		comments = parser.getComments( fixture );
 		parser.groupThreads( comments );
 
 		// Add a reply to every comment. Note that this inserts *all* of the replies, unlike the real
 		// thing, which only deals with one at a time. This isn't ideal but resetting everything after
 		// every reply would be super slow.
+		nodes = [];
 		for ( j = 0; j < comments.length; j++ ) {
 			if ( comments[ j ].type === 'heading' ) {
 				continue;
 			}
 			node = modifier.addListItem( comments[ j ] );
 			node.textContent = 'Reply to ' + comments[ j ].id;
+			nodes.push( node );
 		}
 
 		// Uncomment this to get updated content for the the "modified HTML" files, for copy/paste:
@@ -87,5 +93,17 @@ QUnit.test( '#addListItem', function ( assert ) {
 			expectedHtml,
 			cases[ i ].name
 		);
+
+		// Now discard the replies and verify we get the original document back.
+		for ( j = 0; j < nodes.length; j++ ) {
+			modifier.removeListItem( nodes[ j ] );
+		}
+
+		reverseActualHtml = fixture.innerHTML;
+		assert.strictEqual(
+			reverseActualHtml,
+			reverseExpectedHtml,
+			cases[ i ].name + ' (discard replies)'
+		);
 	}
 } );