From 8effb3f2185d0e6f08dbccb492ff9e3f2a06ac35 Mon Sep 17 00:00:00 2001
From: Robin Munn <rmunn@pobox.com>
Date: Mon, 25 Sep 2023 02:19:50 +0700
Subject: [PATCH] Fix list widget bug with counter-last when appending items
 (#7712)

* Add failing test for list widget with counter-last

The failing test appends a value to a list without changing the rest of
the list, and the counter-last value doesn't get updated correctly when
that happens. Also added another test, which passes, testing removing
the last item of the list, just in case of a regression.

* Improve unit tests for counter-last list widget bug

The unit tests were looking very similar to each other, so I factored
out the common code and made them into simple data-driven tests.

* Fix bug where counter-last fails in list widget

The only scenario that was failing was when counter-last was used, but
the list was strictly appended to with no other changes made. The one
unit test that was failing now passes with this fix.

* Improve bugfix to list widget counter-last

Now we only refresh the last item if it was truly necessary.
---
 core/modules/widgets/list.js                | 11 ++++++++++
 editions/test/tiddlers/tests/test-widget.js | 23 +++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/core/modules/widgets/list.js b/core/modules/widgets/list.js
index 41344a02e..75592e669 100755
--- a/core/modules/widgets/list.js
+++ b/core/modules/widgets/list.js
@@ -225,6 +225,8 @@ ListWidget.prototype.handleListChanges = function(changedTiddlers) {
 		// If we are providing an counter variable then we must refresh the items, otherwise we can rearrange them
 		var hasRefreshed = false,t;
 		if(this.counterName) {
+			var mustRefreshOldLast = false;
+			var oldLength = this.children.length;
 			// Cycle through the list and remove and re-insert the first item that has changed, and all the remaining items
 			for(t=0; t<this.list.length; t++) {
 				if(hasRefreshed || !this.children[t] || this.children[t].parseTreeNode.itemTitle !== this.list[t]) {
@@ -232,6 +234,9 @@ ListWidget.prototype.handleListChanges = function(changedTiddlers) {
 						this.removeListItem(t);
 					}
 					this.insertListItem(t,this.list[t]);
+					if(!hasRefreshed && t === oldLength) {
+						mustRefreshOldLast = true;
+					}
 					hasRefreshed = true;
 				} else {
 					// Refresh the item we're reusing
@@ -239,6 +244,12 @@ ListWidget.prototype.handleListChanges = function(changedTiddlers) {
 					hasRefreshed = hasRefreshed || refreshed;
 				}
 			}
+			// If items were inserted then we must recreate the item that used to be at the last position as it is no longer last
+			if(mustRefreshOldLast && oldLength > 0) {
+				var oldLastIdx = oldLength-1;
+				this.removeListItem(oldLastIdx);
+				this.insertListItem(oldLastIdx,this.list[oldLastIdx]);
+			}
 			// If there are items to remove and we have not refreshed then recreate the item that will now be at the last position
 			if(!hasRefreshed && this.children.length > this.list.length) {
 				this.removeListItem(this.list.length-1);
diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js
index 544ed928f..4da9e20b0 100755
--- a/editions/test/tiddlers/tests/test-widget.js
+++ b/editions/test/tiddlers/tests/test-widget.js
@@ -527,6 +527,29 @@ describe("Widget module", function() {
 		expect(wrapper.children[0].children[15].sequenceNumber).toBe(53);
 	});
 
+	var testCounterLast = function(oldList, newList) {
+		return function() {
+			var wiki = new $tw.Wiki();
+			// Add some tiddlers
+			wiki.addTiddler({title: "Numbers", text: "", list: oldList});
+			var text = "<$list filter='[list[Numbers]]' variable='item' counter='c'><<item>><$text text={{{ [<c-last>match[no]then[, ]] }}} /></$list>";
+			var widgetNode = createWidgetNode(parseText(text,wiki),wiki);
+			// Render the widget node to the DOM
+			var wrapper = renderWidgetNode(widgetNode);
+			// Test the rendering
+			expect(wrapper.innerHTML).toBe("<p>" + oldList.split(' ').join(', ') + "</p>");
+			// Append a number
+			wiki.addTiddler({title: "Numbers", text: "", list: newList});
+			refreshWidgetNode(widgetNode,wrapper,["Numbers"]);
+			expect(wrapper.innerHTML).toBe("<p>" + newList.split(' ').join(', ') + "</p>");
+		}
+	}
+
+	it("the list widget with counter-last should update correctly when list is appended", testCounterLast("1 2 3 4", "1 2 3 4 5"));
+	it("the list widget with counter-last should update correctly when last item is removed", testCounterLast("1 2 3 4", "1 2 3"));
+	it("the list widget with counter-last should update correctly when first item is inserted", testCounterLast("1 2 3 4", "0 1 2 3 4"));
+	it("the list widget with counter-last should update correctly when first item is removed", testCounterLast("1 2 3 4", "2 3 4"));
+
 	it("should deal with the list widget followed by other widgets", function() {
 		var wiki = new $tw.Wiki();
 		// Add some tiddlers