Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions packages/cells/src/cells/multi-select-cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
} from "@glideapps/glide-data-grid";

import { styled } from "@linaria/react";
import Select, { type MenuProps, components, type StylesConfig } from "react-select";
import Select, { type MenuProps, type MultiValueGenericProps, components, type StylesConfig } from "react-select";
import CreatableSelect from "react-select/creatable";

type SelectOption = { value: string; label?: string; color?: string };
Expand Down Expand Up @@ -128,6 +128,48 @@ const CustomMenu: React.FC<CustomMenuProps> = p => {
return <Menu {...rest}>{children}</Menu>;
};

/**
* Custom MultiValueLabel component that allows text selection within pills.
* By default, react-select prevents text selection via onMouseDown preventDefault.
* We override this to allow users to select and copy text from the pills.
*
* Side effects:
* - Clicking on the pill label text won't focus the select input (click elsewhere to focus)
* - Clicking on the pill label text won't open the dropdown menu (click input area to open)
* - Removing pills via the X button still works normally (separate component)
* - Keyboard navigation still works normally
*
* Note on type assertions: react-select's MultiValueGenericProps.innerProps type is
* { className?: string }, but the underlying div element accepts all standard div props.
* The type assertion to React.ComponentPropsWithoutRef<"div"> is necessary to add
* event handlers that the actual DOM element supports.
*/
const SelectableMultiValueLabel: React.FC<MultiValueGenericProps<SelectOption>> = props => {
// Cast innerProps to the full div props type since react-select's types are overly restrictive
// (they only type { className?: string } but the div accepts all standard props)
const existingInnerProps = props.innerProps as React.ComponentPropsWithoutRef<"div"> | undefined;

const enhancedInnerProps: React.ComponentPropsWithoutRef<"div"> = {
...existingInnerProps,
// Allow text selection by stopping propagation but not preventing default
onMouseDown: (e: React.MouseEvent<HTMLDivElement>) => {
e.stopPropagation(); // Prevents react-select from treating it as a control click
existingInnerProps?.onMouseDown?.(e);
},
onTouchEnd: (e: React.TouchEvent<HTMLDivElement>) => {
e.stopPropagation();
existingInnerProps?.onTouchEnd?.(e);
},
};

return (
<components.MultiValueLabel
{...props}
innerProps={enhancedInnerProps as typeof props.innerProps}
/>
);
};
Comment thread
lukasmasuch marked this conversation as resolved.

export type MultiSelectCell = CustomCell<MultiSelectCellProps>;

const Editor: ReturnType<ProvideEditorCallback<MultiSelectCell>> = p => {
Expand Down Expand Up @@ -367,6 +409,7 @@ const Editor: ReturnType<ProvideEditorCallback<MultiSelectCell>> = p => {
components={{
DropdownIndicator: () => null,
IndicatorSeparator: () => null,
MultiValueLabel: SelectableMultiValueLabel,
Menu: props => {
if (menuDisabled) {
return null;
Expand All @@ -379,10 +422,10 @@ const Editor: ReturnType<ProvideEditorCallback<MultiSelectCell>> = p => {
},
}}
onChange={async e => {
if (e === null) {
if (e === null || !Array.isArray(e)) {
return;
}
submitValues(e.map(x => x.value));
submitValues(e.map((x: SelectOption) => x.value));
}}
/>
</Wrap>
Expand Down
95 changes: 94 additions & 1 deletion packages/cells/test/multi-select-cell.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ describe("Multi Select Editor", () => {
const Editor = renderer.provideEditor?.({
...getMockCell(),
location: [0, 0],
}).editor;
}).editor;
if (Editor === undefined) {
throw new Error("Editor is invalid");
}
Expand Down Expand Up @@ -377,4 +377,97 @@ describe("Multi Select Editor", () => {
});

// TODO: Add test for creating new options

it("allows text selection in pill labels (onMouseDown does not prevent default)", async () => {
const mockCell = getMockCell({
data: {
kind: "multi-select-cell",
options: [
{ value: "option1", label: "Option 1", color: "red" },
{ value: "option2", label: "Option 2", color: "blue" },
],
values: ["option1", "option2"],
},
});
// @ts-ignore
const Editor = renderer.provideEditor?.({
...mockCell,
location: [0, 0],
}).editor;
if (Editor === undefined) {
throw new Error("Editor is invalid");
}

const mockCellOnChange = vi.fn();
const result = render(<Editor isHighlighted={false} value={mockCell} onChange={mockCellOnChange} />);
const cellEditor = result.getByTestId("multi-select-cell");

// Find the pill labels (MultiValueLabel components render with the label text)
const pillLabel = getByText(cellEditor, "Option 1");
expect(pillLabel).toBeDefined();

// Simulate mousedown on the pill label - it should not prevent default (allowing text selection)
// We verify this by checking that the event's defaultPrevented is false after the handler runs
const mouseDownEvent = new MouseEvent("mousedown", {
bubbles: true,
cancelable: true,
});

// The event should not be prevented (allowing text selection)
pillLabel.dispatchEvent(mouseDownEvent);
expect(mouseDownEvent.defaultPrevented).toBe(false);

// The onChange should NOT have been called just from clicking the label
// (stopPropagation prevents the control from receiving the click)
expect(mockCellOnChange).not.toHaveBeenCalled();
});

it("still allows removing pills via the remove button after text selection enhancement", async () => {
const mockCell = getMockCell({
data: {
kind: "multi-select-cell",
options: [
{ value: "option1", label: "Option 1", color: "red" },
{ value: "option2", label: "Option 2", color: "blue" },
],
values: ["option1"],
},
});
// @ts-ignore
const Editor = renderer.provideEditor?.({
...mockCell,
location: [0, 0],
}).editor;
if (Editor === undefined) {
throw new Error("Editor is invalid");
}

const mockCellOnChange = vi.fn();
const result = render(<Editor isHighlighted={false} value={mockCell} onChange={mockCellOnChange} />);
const cellEditor = result.getByTestId("multi-select-cell");

// Find the pill label first
const pillLabel = getByText(cellEditor, "Option 1");
expect(pillLabel).toBeDefined();

// The remove button is a sibling of the label within the multi-value container
// react-select renders: <div class="...multi-value"><div class="...label">text</div><div class="...remove">X</div></div>
const multiValueContainer = pillLabel.parentElement;
expect(multiValueContainer).not.toBeNull();

// Find the remove button (it's the element with the SVG/X icon, typically the last child or has a specific role)
// react-select's remove button contains an SVG with a path
const removeButton = multiValueContainer?.querySelector("svg")?.parentElement;
expect(removeButton).not.toBeNull();

// Click the remove button
fireEvent.click(removeButton!);

// The onChange should have been called to remove the value
expect(mockCellOnChange).toHaveBeenCalledTimes(1);
expect(mockCellOnChange).toHaveBeenCalledWith({
...mockCell,
data: { ...mockCell.data, values: [] },
});
});
});