Merge pull request #4862 from m3ftwz/fix-multiselect-list

Fix multiselect list editor duplicate selections.

Author: azmy60

src/js/modules/Edit/List.js

@@ -410,7 +410,11 @@ export default class Edit{
 			this._resolveValue(true);
 		}else{
 			if(this.focusedItem){
-				this._chooseItem(this.focusedItem);
+				if(this.isFilter && !this.params.multiselect && this.focusedItem.selected){
+					this._resolveValue();
+				}else{
+					this._chooseItem(this.focusedItem);
+				}
 			}
 		}
 	}
@@ -655,6 +659,10 @@ export default class Edit{
 			this.lastAction = "typing";
 		}
 
+		if(this.params.multiselect) {
+			this.initialValues = null;
+		}
+		
 		this.data = data;
 
 		return data;    
@@ -678,7 +686,19 @@ export default class Edit{
 				original:option,
 			};
 
-			if(this.initialValues && this.initialValues.indexOf(option.value) > -1){
+			if(this.params.multiselect){
+				var existingIndex = this.currentItems.findIndex(existing => existing.value === option.value);
+				if(existingIndex > -1){
+					if(this.focusedItem === this.currentItems[existingIndex]){
+						this.focusedItem = item;
+					}
+					
+					this.currentItems[existingIndex] = item;
+					item.selected = true;
+				}else if(this.initialValues && this.initialValues.indexOf(option.value) > -1){
+					this._chooseItem(item, true);
+				}
+			}else if(this.initialValues && this.initialValues.indexOf(option.value) > -1){
 				this._chooseItem(item, true);
 			}
 		}
@@ -997,6 +1017,12 @@ export default class Edit{
 			this._styleItem(item);
 
 		}else{
+			if(this.isFilter && !silent && item.selected){
+				this._clearChoices();
+				this.input.value = "";
+				this._resolveValue();
+				return;
+			}
 			this.currentItems = [item];
 			item.selected = true;
 
@@ -1032,6 +1058,8 @@ export default class Edit{
 			}else{
 				if(this.currentItems[0]){
 					output = this.currentItems[0].value;
+				}else if(this.isFilter && this.focusedItem && this.focusedItem.selected){
+					output = this.focusedItem.value;
 				}else{
 					initialValue = Array.isArray(this.initialValues) ? this.initialValues[0] : this.initialValues;
 

test/unit/modules/Edit.spec.js

@@ -1,5 +1,6 @@
 import TabulatorFull from '../../../src/js/core/TabulatorFull.js';
 import Edit from '../../../src/js/modules/Edit/Edit.js';
+import List from '../../../src/js/modules/Edit/List.js';
 import { DateTime } from "luxon";
 
 describe("Edit module", () => {
@@ -34,7 +35,38 @@ describe("Edit module", () => {
 		return newTable;
 	};
 
+	const setupList = async ({
+		cellType = "header",
+		cellValue = "Female",
+		editorParams = { values: ["Male", "Female"] },
+		success = jest.fn(),
+		cancel = jest.fn(),
+	} = {}) => {
+		const element = document.createElement("div");
+		const list = new List(
+			{
+				table: {},
+			},
+			{
+				getType: jest.fn().mockReturnValue(cellType),
+				getValue: jest.fn().mockReturnValue(cellValue),
+				getElement: jest.fn().mockReturnValue(element),
+			},
+			jest.fn(),
+			success,
+			cancel,
+			editorParams
+		);
+
+		list.input.dispatchEvent(new Event("focus"));
+		await new Promise(resolve => setTimeout(resolve, 0));
+
+		return {list, success, cancel, element};
+	};
+	
 	beforeEach(async () => {
+		// jsdom does not implement scrollIntoView, which the list focus uses.
+		Element.prototype.scrollIntoView = jest.fn();
 		table = await setupTable();
 	});
 
@@ -126,6 +158,61 @@ describe("Edit module", () => {
 		// Should have received the cellEdited event
 		expect(cellEditedSpy).toHaveBeenCalled();
 	});
+	
+	it("should apply a focused header list filter item when Enter is pressed", async () => {
+		const {list, success} = await setupList({
+			cellValue: "",
+		});
+
+		// Navigate down to "Female" (the second option) and apply it.
+		list.listEl.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true }));
+		list.listEl.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true }));
+		list.listEl.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true }));
+
+		expect(success).toHaveBeenCalledWith("Female");
+		expect(list.input.value).toBe("Female");
+	});
+
+	it("should keep a header list filter selected when Enter is pressed repeatedly", async () => {
+		const {list, success} = await setupList();
+
+		list.listEl.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true }));
+		list.listEl.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true }));
+
+		expect(success).toHaveBeenCalledTimes(2);
+		expect(success).toHaveBeenNthCalledWith(1, "Female");
+		expect(success).toHaveBeenNthCalledWith(2, "Female");
+	});
+
+	it("should use currentItems instead of initialValues after the first multiselect parse", async () => {
+		const {list} = await setupList({
+			cellType: "cell",
+			cellValue: ["red", "blue"],
+			editorParams: {
+				multiselect: true,
+				values: ["red", "green", "blue"],
+			},
+		});
+
+		expect(list.initialValues).toBeNull();
+		expect(list.currentItems.map(item => item.value)).toEqual(["red", "blue"]);
+
+		// Deselect "red" by clicking its rendered list item.
+		list.data.find(item => item.value === "red").element.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+
+		expect(list.currentItems.map(item => item.value)).toEqual(["blue"]);
+
+		list.input.dispatchEvent(new Event("focus"));
+		await new Promise(resolve => setTimeout(resolve, 0));
+
+		const redItem = list.data.find(item => item.value === "red");
+		const blueItem = list.data.find(item => item.value === "blue");
+
+		expect(list.initialValues).toBeNull();
+		expect(redItem.selected).toBe(false);
+		expect(blueItem.selected).toBe(true);
+		expect(list.currentItems).toEqual([blueItem]);
+	});
 
 	// Regression test for https://github.com/tabulator-tables/tabulator/issues/4421
 	//
@@ -268,4 +355,4 @@ describe("Edit module - date editors with 'x' (epoch milliseconds) format", () =
 
 		expect(typeof cell.getValue()).toBe("number");
 	});
-});
+});