Ensure clicking on a cell once enables editing mode (#10365)

* modify cell click behaviour * fix mouse pointer events * add changeset * deselect cell on click outside * ensure cell menu closes * tweak editable logic * update e2e test with single click --------- Co-authored-by: gradio-pr-bot <[email protected]> Co-authored-by: Abubakar Abid <[email protected]>
gradio-app · Jan 17, 2025 · 40e0c48 · 40e0c48
1 parent b0cf92f
commit 40e0c48
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 78 deletions.
diff --git a/.changeset/five-crabs-stop.md b/.changeset/five-crabs-stop.md
@@ -0,0 +1,6 @@
+---
+"@gradio/dataframe": patch
+"gradio": patch
+---
+
+fix:Ensure clicking on a cell once enables editing mode
diff --git a/js/dataframe/shared/EditableCell.svelte b/js/dataframe/shared/EditableCell.svelte
@@ -20,7 +20,6 @@
 		display: boolean;
 	}[];
 	export let clear_on_focus = false;
-	export let select_on_focus = false;
 	export let line_breaks = true;
 	export let editable = true;
 	export let root: string;
@@ -34,11 +33,10 @@
 		if (clear_on_focus) {
 			_value = "";
 		}
-		if (select_on_focus) {
-			node.select();
-		}
 
-		node.focus();
+		requestAnimationFrame(() => {
+			node.focus();
+		});
 
 		return {};
 	}
@@ -69,6 +67,9 @@
 		class:header
 		tabindex="-1"
 		on:blur={handle_blur}
+		on:mousedown|stopPropagation
+		on:mouseup|stopPropagation
+		on:click|stopPropagation
 		use:use_focus
 		on:keydown={handle_keydown}
 	/>
@@ -81,6 +82,8 @@
 	class:edit
 	on:focus|preventDefault
 	style={styling}
+	class="table-cell-text"
+	placeholder=" "
 >
 	{#if datatype === "html"}
 		{@html value}
@@ -123,7 +126,7 @@
 
 	.header {
 		transform: translateX(0);
-		font: var(--weight-bold);
+		font-weight: var(--weight-bold);
 	}
 
 	.edit {

diff --git a/js/dataframe/shared/Table.svelte b/js/dataframe/shared/Table.svelte
@@ -38,6 +38,7 @@
 	export let stream_handler: Client["stream"];
 
 	let selected: false | [number, number] = false;
+	let clicked_cell: { row: number; col: number } | undefined = undefined;
 	export let display_value: string[][] | null = null;
 	export let styling: string[][] | null = null;
 	let t_rect: DOMRectReadOnly;
@@ -204,12 +205,6 @@
 		);
 	}
 
-	async function start_edit(i: number, j: number): Promise<void> {
-		if (!editable || dequal(editing, [i, j])) return;
-
-		editing = [i, j];
-	}
-
 	function move_cursor(
 		key: "ArrowRight" | "ArrowLeft" | "ArrowDown" | "ArrowUp",
 		current_coords: [number, number]
@@ -349,25 +344,6 @@
 		}
 	}
 
-	let active_cell: { row: number; col: number } | null = null;
-
-	async function handle_cell_click(i: number, j: number): Promise<void> {
-		if (active_cell && active_cell.row === i && active_cell.col === j) {
-			active_cell = null;
-		} else {
-			active_cell = { row: i, col: j };
-		}
-		if (dequal(editing, [i, j])) return;
-		header_edit = false;
-		selected_header = false;
-		editing = false;
-		if (!dequal(selected, [i, j])) {
-			selected = [i, j];
-			await tick();
-			parent.focus();
-		}
-	}
-
 	type SortDirection = "asc" | "des";
 	let sort_direction: SortDirection | undefined;
 	let sort_by: number | undefined;
@@ -479,17 +455,16 @@
 			active_header_menu = null;
 		}
 
-		event.stopImmediatePropagation();
 		const [trigger] = event.composedPath() as HTMLElement[];
 		if (parent.contains(trigger)) {
 			return;
 		}
 
+		clicked_cell = undefined;
 		editing = false;
+		selected = false;
 		header_edit = false;
 		selected_header = false;
-		reset_selection();
-		active_cell = null;
 		active_cell_menu = null;
 		active_header_menu = null;
 	}
@@ -779,18 +754,9 @@
 			}
 		}
 	}
-
-	function reset_selection(): void {
-		selected = false;
-		last_selected = null;
-	}
 </script>
 
-<svelte:window
-	on:click={handle_click_outside}
-	on:touchstart={handle_click_outside}
-	on:resize={() => set_cell_widths()}
-/>
+<svelte:window on:resize={() => set_cell_widths()} />
 
 <div class:label={label && label.length !== 0} use:copy>
 	{#if label && label.length !== 0 && show_label}
@@ -916,17 +882,17 @@
 										edit={header_edit === i}
 										on:keydown={end_header_edit}
 										on:dblclick={() => edit_header(i)}
-										{select_on_focus}
 										header
 										{root}
 									/>
 									<!-- TODO: fix -->
 									<!-- svelte-ignore a11y-click-events-have-key-events -->
-									<!-- svelte-ignore a11y-no-static-element-interactions-->
 									<div
 										class:sorted={sort_by === i}
 										class:des={sort_by === i && sort_direction === "des"}
 										class="sort-button {sort_direction}"
+										role="button"
+										tabindex="0"
 										on:click={(event) => {
 											event.stopPropagation();
 											handle_sort(i);
@@ -961,12 +927,34 @@
 					{#each item as { value, id }, j (id)}
 						<td
 							tabindex="0"
-							on:touchstart={() => start_edit(index, j)}
-							on:click={() => {
-								handle_cell_click(index, j);
+							on:touchstart={(event) => {
+								event.preventDefault();
+								event.stopPropagation();
+								clear_on_focus = false;
+								clicked_cell = { row: index, col: j };
+								selected = [index, j];
+								if (editable) {
+									editing = [index, j];
+								}
+								toggle_cell_button(index, j);
+							}}
+							on:mousedown={(event) => {
+								event.preventDefault();
+								event.stopPropagation();
+							}}
+							on:click={(event) => {
+								event.preventDefault();
+								event.stopPropagation();
+								clear_on_focus = false;
+								active_cell_menu = null;
+								active_header_menu = null;
+								clicked_cell = { row: index, col: j };
+								selected = [index, j];
+								if (editable) {
+									editing = [index, j];
+								}
 								toggle_cell_button(index, j);
 							}}
-							on:dblclick={() => start_edit(index, j)}
 							style:width="var(--cell-width-{j})"
 							style={styling?.[index]?.[j] || ""}
 							class:focus={dequal(selected, [index, j])}
@@ -984,7 +972,10 @@
 									{editable}
 									edit={dequal(editing, [index, j])}
 									datatype={Array.isArray(datatype) ? datatype[j] : datatype}
-									on:blur={() => ((clear_on_focus = false), parent.focus())}
+									on:blur={() => {
+										clear_on_focus = false;
+										parent.focus();
+									}}
 									{clear_on_focus}
 									{root}
 								/>

diff --git a/js/spa/test/blocks_flashcards.spec.ts b/js/spa/test/blocks_flashcards.spec.ts
@@ -6,38 +6,18 @@ test("shows the results tab when results > 0", async ({ page }) => {
 		page.getByText("Please enter word prompts into the table.")
 	).toBeAttached();
 	await page.getByLabel("Close").click();
-
 	await page
 		.getByRole("button", { name: "front back" })
-		.getByRole("button")
+		.locator("span")
 		.nth(2)
-		.dblclick();
-	await page
-		.getByRole("button", { name: "front back" })
-		.locator("tbody")
-		.getByRole("textbox")
-		.fill("dog");
-	await page
-		.getByRole("button", { name: "front back" })
-		.locator("tbody")
-		.getByRole("textbox")
-		.press("Enter");
-
+		.click();
+	await page.getByRole("textbox").fill("dog");
 	await page
-		.getByRole("button", { name: "front back" })
+		.getByRole("row", { name: "dog ⋮" })
 		.getByRole("button")
-		.nth(4)
-		.dblclick();
-	await page
-		.getByRole("button", { name: "front back" })
-		.locator("tbody")
-		.getByRole("textbox")
-		.fill("cat");
-	await page
-		.getByRole("button", { name: "front back" })
-		.locator("tbody")
-		.getByRole("textbox")
-		.press("Enter");
+		.nth(2)
+		.click();
+	await page.getByRole("textbox").fill("cat");
 
 	await page.getByText("Start Practice").dblclick();
 });