Skip to content

Commit 362a7e7

Browse files
authored
fix: allow select on table cells (#5565)
## 📝 Summary <!-- Provide a concise summary of what this pull request is addressing. If this PR fixes any issues, list them here by number (e.g., Fixes #123). --> https://github.com/user-attachments/assets/92337c8d-1250-4bc1-92b4-e3291f3fae5b ## 🔍 Description of Changes <!-- Detail the specific changes made in this pull request. Explain the problem addressed and how it was resolved. If applicable, provide before and after comparisons, screenshots, or any relevant details to help reviewers understand the changes easily. --> ## 📋 Checklist - [x] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [ ] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [ ] I have added tests for the changes made. - [ ] I have run the code and verified that it works as expected. ## 📜 Reviewers <!-- Tag potential reviewers from the community or maintainers who might be interested in reviewing this pull request. Your PR will be reviewed more quickly if you can figure out the right person to tag with @ @mscolnick (General, AI) @dmadisetti (Runtime, Caching, Fileformat, AST) @manzt (Widgets, Dependency Management, LSP) @Light2Dark (Tables, Plots, Layouts, SQL) @akshayka (Public API, Dependencies, UX/Styling, Backend, Docs, Integrations) -->
1 parent ed77c43 commit 362a7e7

File tree

3 files changed

+48
-2
lines changed

3 files changed

+48
-2
lines changed

frontend/src/components/data-table/range-focus/__tests__/atoms.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
22

33
import type { Cell, Row, Table } from "@tanstack/react-table";
4-
import { beforeEach, describe, expect, it, vi } from "vitest";
4+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
55
import { visibleForTesting } from "../atoms";
66

77
// Mock dependencies
@@ -406,6 +406,11 @@ describe("cell selection atoms", () => {
406406
vi.mocked(getCellValues).mockReturnValue("mocked cell values");
407407
});
408408

409+
afterEach(() => {
410+
// Clear text selection
411+
window.getSelection = vi.fn().mockReturnValue(null);
412+
});
413+
409414
it("should copy selected cells and call onCopyComplete", () => {
410415
const selectedCells = new Set(["row1_col1", "row1_col2"]);
411416
const onCopyComplete = vi.fn();
@@ -423,6 +428,29 @@ describe("cell selection atoms", () => {
423428
expect(onCopyComplete).toHaveBeenCalledWith();
424429
expect(state.copiedCells).toEqual(selectedCells);
425430
});
431+
432+
it("should not copy if there is text selection", () => {
433+
const selectedCells = new Set(["row1_col1", "row1_col2"]);
434+
const onCopyComplete = vi.fn();
435+
436+
// Set some selected cells first
437+
actions.setSelectedCells(selectedCells);
438+
439+
// Set text selection
440+
window.getSelection = vi.fn().mockReturnValue({
441+
toString: vi.fn().mockReturnValue("some text"),
442+
});
443+
444+
actions.handleCopy({
445+
table: mockTable,
446+
onCopyComplete,
447+
});
448+
449+
expect(getCellValues).not.toHaveBeenCalled();
450+
expect(copyToClipboard).not.toHaveBeenCalled();
451+
expect(onCopyComplete).not.toHaveBeenCalled();
452+
expect(state.copiedCells).toEqual(new Set());
453+
});
426454
});
427455

428456
describe("navigate", () => {

frontend/src/components/data-table/range-focus/atoms.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ const {
141141
selectedCell,
142142
);
143143

144+
const isSelectingMultipleCells = cellsInRange.length > 1;
145+
if (isSelectingMultipleCells) {
146+
clearTextSelection();
147+
}
148+
144149
return {
145150
...state,
146151
selectedCells: new Set(cellsInRange),
@@ -156,6 +161,12 @@ const {
156161
onCopyComplete: () => void;
157162
},
158163
) => {
164+
const selection = window.getSelection();
165+
if (selection && selection.toString().length > 0) {
166+
// Let browser handle the copy
167+
return state;
168+
}
169+
159170
const text = getCellValues(table, state.selectedCells);
160171
copyToClipboard(text);
161172
onCopyComplete();
@@ -360,3 +371,10 @@ export const createCellStateAtom = (cellId: string) =>
360371
isCopied: copiedCells.has(cellId),
361372
};
362373
});
374+
375+
// Clear browser text selection to prevent visual conflicts with cell selection indicator
376+
export const clearTextSelection = () => {
377+
if (window.getSelection) {
378+
window.getSelection()?.empty();
379+
}
380+
};

frontend/src/components/data-table/renderers.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ export const DataTableBody = <TData,>({
101101
tabIndex={0}
102102
key={cell.id}
103103
className={cn(
104-
"whitespace-pre truncate max-w-[300px] select-none outline-none",
104+
"whitespace-pre truncate max-w-[300px] outline-none",
105105
cell.column.getColumnWrapping &&
106106
cell.column.getColumnWrapping() === "wrap" &&
107107
"whitespace-pre-wrap min-w-[200px]",

0 commit comments

Comments
 (0)