Skip to content

Commit aa3a7f6

Browse files
authored
feat: updates to row viewer panel (#4866)
## 📝 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). --> ![image](https://github.com/user-attachments/assets/c31e3273-af1a-42e7-b4ae-f6124f1cad5c) - Change to Row viewer panel - Adds cell-aware config - Keyboard shortcuts when element is active open to name/tooltip suggestions. Context-aware(?), reactive, adaptive ## 🔍 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. - [X] 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
1 parent 50d9914 commit aa3a7f6

File tree

11 files changed

+222
-80
lines changed

11 files changed

+222
-80
lines changed

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ interface TableActionsProps<TData> {
2727
getRowIds?: GetRowIds;
2828
toggleDisplayHeader?: () => void;
2929
chartsFeatureEnabled?: boolean;
30-
toggleSelectionPanel?: () => void;
31-
isSelectionPanelOpen?: boolean;
30+
toggleRowViewerPanel?: () => void;
31+
isRowViewerPanelOpen?: boolean;
3232
}
3333

3434
export const TableActions = <TData,>({
@@ -45,8 +45,8 @@ export const TableActions = <TData,>({
4545
getRowIds,
4646
toggleDisplayHeader,
4747
chartsFeatureEnabled,
48-
toggleSelectionPanel,
49-
isSelectionPanelOpen,
48+
toggleRowViewerPanel,
49+
isRowViewerPanelOpen,
5050
}: TableActionsProps<TData>) => {
5151
const handleSelectAllRows = (value: boolean) => {
5252
if (!onRowSelectionChange) {
@@ -118,13 +118,13 @@ export const TableActions = <TData,>({
118118
</Button>
119119
</Tooltip>
120120
)}
121-
{toggleSelectionPanel && (
122-
<Tooltip content="Toggle selection panel">
123-
<Button variant="text" size="xs" onClick={toggleSelectionPanel}>
121+
{toggleRowViewerPanel && (
122+
<Tooltip content="Toggle row viewer">
123+
<Button variant="text" size="xs" onClick={toggleRowViewerPanel}>
124124
<PanelRightIcon
125125
className={cn(
126126
"w-4 h-4 text-muted-foreground",
127-
isSelectionPanelOpen && "text-primary",
127+
isRowViewerPanelOpen && "text-primary",
128128
)}
129129
/>
130130
</Button>

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ interface DataTableProps<TData> extends Partial<DownloadActionProps> {
7979
onFocusRowChange?: OnChangeFn<number>;
8080
// Others
8181
chartsFeatureEnabled?: boolean;
82-
toggleSelectionPanel?: () => void;
83-
isSelectionPanelOpen?: boolean;
82+
toggleRowViewerPanel?: () => void;
83+
isRowViewerPanelOpen?: boolean;
8484
}
8585

8686
const DataTableInternal = <TData,>({
@@ -116,8 +116,8 @@ const DataTableInternal = <TData,>({
116116
freezeColumnsRight,
117117
toggleDisplayHeader,
118118
chartsFeatureEnabled,
119-
toggleSelectionPanel,
120-
isSelectionPanelOpen,
119+
toggleRowViewerPanel,
120+
isRowViewerPanelOpen,
121121
onFocusRowChange,
122122
}: DataTableProps<TData>) => {
123123
const [isSearchEnabled, setIsSearchEnabled] = React.useState<boolean>(false);
@@ -229,7 +229,7 @@ const DataTableInternal = <TData,>({
229229
{renderTableBody(
230230
table,
231231
columns,
232-
isSelectionPanelOpen,
232+
isRowViewerPanelOpen,
233233
getPaginatedRowIndex,
234234
)}
235235
</Table>
@@ -248,8 +248,8 @@ const DataTableInternal = <TData,>({
248248
getRowIds={getRowIds}
249249
toggleDisplayHeader={toggleDisplayHeader}
250250
chartsFeatureEnabled={chartsFeatureEnabled}
251-
toggleSelectionPanel={toggleSelectionPanel}
252-
isSelectionPanelOpen={isSelectionPanelOpen}
251+
toggleRowViewerPanel={toggleRowViewerPanel}
252+
isRowViewerPanelOpen={isRowViewerPanelOpen}
253253
/>
254254
</div>
255255
);

frontend/src/components/data-table/selection-panel/__tests__/filter-rows.test.ts renamed to frontend/src/components/data-table/row-viewer-panel/__tests__/filter-rows.test.ts

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

33
import { describe, expect, it } from "vitest";
4-
import { filterRows } from "../data-selection";
4+
import { filterRows } from "../row-viewer";
55

66
describe("filterRows", () => {
77
const defaultRowValues = {

frontend/src/components/data-table/selection-panel/__tests__/data-selection.test.tsx renamed to frontend/src/components/data-table/row-viewer-panel/__tests__/row-viewer.test.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { describe, expect, it, vi } from "vitest";
44
import { render, screen } from "@testing-library/react";
5-
import { DataSelectionPanel } from "../data-selection";
5+
import { RowViewerPanel } from "../row-viewer";
66
import { Provider } from "jotai";
77
import { TooltipProvider } from "@/components/ui/tooltip";
88
import type { GetRowResult } from "@/plugins/impl/DataTablePlugin";
@@ -16,7 +16,7 @@ const renderWithProviders = (component: React.ReactNode) => {
1616
);
1717
};
1818

19-
describe("DataSelectionPanel", () => {
19+
describe("RowExpandedPanel", () => {
2020
const mockFieldTypes: FieldTypesWithExternalType = [
2121
["name", ["string", "string"]],
2222
["age", ["number", "number"]],
@@ -35,9 +35,9 @@ describe("DataSelectionPanel", () => {
3535

3636
const mockSetRowIdx = vi.fn();
3737

38-
it("renders data in selection panel", async () => {
38+
it("renders data in row viewer panel", async () => {
3939
renderWithProviders(
40-
<DataSelectionPanel
40+
<RowViewerPanel
4141
rowIdx={0}
4242
setRowIdx={mockSetRowIdx}
4343
totalRows={3}

frontend/src/components/data-table/selection-panel/data-selection.tsx renamed to frontend/src/components/data-table/row-viewer-panel/row-viewer.tsx

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
import { DATA_TYPE_ICON } from "@/components/datasets/icons";
2222
import { Input } from "@/components/ui/input";
2323
import { CopyClipboardIcon } from "@/components/icons/copy-icon";
24-
import { useState } from "react";
24+
import { useState, useRef } from "react";
2525
import { useAsyncData } from "@/hooks/useAsyncData";
2626
import {
2727
INDEX_COLUMN_NAME,
@@ -34,23 +34,26 @@ import { NAMELESS_COLUMN_PREFIX } from "../columns";
3434
import { Banner, ErrorBanner } from "@/plugins/impl/common/error-banner";
3535
import type { Column } from "@tanstack/react-table";
3636
import { renderCellValue } from "../columns";
37+
import { useKeydownOnElement } from "@/hooks/useHotkey";
3738

38-
export interface DataSelectionPanelProps {
39+
export interface RowViewerPanelProps {
3940
rowIdx: number;
4041
setRowIdx: (rowIdx: number) => void;
4142
totalRows: number;
4243
fieldTypes: FieldTypesWithExternalType | undefined | null;
4344
getRow: (rowIdx: number) => Promise<GetRowResult>;
4445
}
4546

46-
export const DataSelectionPanel: React.FC<DataSelectionPanelProps> = ({
47+
export const RowViewerPanel: React.FC<RowViewerPanelProps> = ({
4748
rowIdx,
4849
setRowIdx,
4950
totalRows,
5051
fieldTypes,
5152
getRow,
52-
}: DataSelectionPanelProps) => {
53+
}: RowViewerPanelProps) => {
5354
const [searchQuery, setSearchQuery] = useState("");
55+
const panelRef = useRef<HTMLDivElement>(null);
56+
const searchInputRef = useRef<HTMLInputElement>(null);
5457

5558
const { data: rows, error } = useAsyncData(async () => {
5659
const data = await getRow(rowIdx);
@@ -64,6 +67,21 @@ export const DataSelectionPanel: React.FC<DataSelectionPanelProps> = ({
6467
setRowIdx(rowIdx);
6568
};
6669

70+
useKeydownOnElement(panelRef, {
71+
ArrowLeft: (e) => {
72+
if (e?.target === searchInputRef.current) {
73+
return false;
74+
}
75+
handleSelectRow(rowIdx - 1);
76+
},
77+
ArrowRight: (e) => {
78+
if (e?.target === searchInputRef.current) {
79+
return false;
80+
}
81+
handleSelectRow(rowIdx + 1);
82+
},
83+
});
84+
6785
const buttonStyles = "h-6 w-6 p-0.5";
6886

6987
const renderTable = () => {
@@ -125,7 +143,7 @@ export const DataSelectionPanel: React.FC<DataSelectionPanelProps> = ({
125143
const filteredRows = filterRows(rowValues, searchQuery);
126144

127145
return (
128-
<Table>
146+
<Table className="mb-4">
129147
<TableHeader>
130148
<TableRow>
131149
<TableHead className="w-1/4">Column</TableHead>
@@ -188,7 +206,11 @@ export const DataSelectionPanel: React.FC<DataSelectionPanelProps> = ({
188206
};
189207

190208
return (
191-
<div className="flex flex-col gap-3 mt-4">
209+
<div
210+
className="flex flex-col gap-3 mt-4 focus:outline-none"
211+
ref={panelRef}
212+
tabIndex={-1}
213+
>
192214
<div className="flex flex-row gap-2 justify-end items-center mr-2">
193215
<Button
194216
variant="outline"
@@ -236,6 +258,7 @@ export const DataSelectionPanel: React.FC<DataSelectionPanelProps> = ({
236258

237259
<div className="mx-2 -mb-1">
238260
<Input
261+
ref={searchInputRef}
239262
type="text"
240263
placeholder="Search"
241264
onChange={(e) => setSearchQuery(e.target.value)}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/* Copyright 2024 Marimo. All rights reserved. */
2+
import { useAtom, useAtomValue } from "jotai";
3+
import { Logger } from "@/utils/Logger";
4+
import type { CellId } from "@/core/cells/ids";
5+
import {
6+
contextAwarePanelOpen,
7+
contextAwarePanelOwner,
8+
isCellAwareAtom,
9+
} from "@/components/editor/chrome/panels/context-aware-panel/atoms";
10+
import { lastFocusedCellIdAtom } from "@/core/cells/focus";
11+
12+
interface PanelOwnershipResult {
13+
isPanelOpen: boolean;
14+
togglePanel: () => void;
15+
}
16+
17+
export function usePanelOwnership(
18+
id: string,
19+
cellId?: CellId | null,
20+
): PanelOwnershipResult {
21+
let isPanelCellAware = useAtomValue(isCellAwareAtom);
22+
const [lastFocusedCellId, setLastFocusedCellId] = useAtom(
23+
lastFocusedCellIdAtom,
24+
);
25+
const [panelOwner, setPanelOwner] = useAtom(contextAwarePanelOwner);
26+
const [isContextAwarePanelOpen, setContextAwarePanelOpen] = useAtom(
27+
contextAwarePanelOpen,
28+
);
29+
const panelId = getPanelId(id, cellId);
30+
31+
if (!cellId && isPanelCellAware) {
32+
Logger.error("CellId is not found, defaulting to fixed mode");
33+
isPanelCellAware = false;
34+
}
35+
36+
const isPanelOpen = panelOwner === panelId && isContextAwarePanelOpen;
37+
38+
const thisCellIsFocused = lastFocusedCellId === cellId;
39+
const currentOwnerIsInThisCell =
40+
panelOwner && isPanelOwner(panelOwner, cellId);
41+
42+
// In cell-aware mode, update panel owner when cell is focused
43+
// Only set panel owner if no other table in this cell is currently the owner
44+
if (
45+
isPanelCellAware &&
46+
thisCellIsFocused &&
47+
panelOwner !== panelId &&
48+
!currentOwnerIsInThisCell
49+
) {
50+
setPanelOwner(panelId);
51+
}
52+
53+
function togglePanel() {
54+
if (isPanelOpen) {
55+
setPanelOwner(null);
56+
setContextAwarePanelOpen(false);
57+
} else {
58+
setPanelOwner(panelId);
59+
// if cell-aware, we want to focus on this cell when toggled open
60+
if (isPanelCellAware && cellId) {
61+
setLastFocusedCellId(cellId);
62+
}
63+
setContextAwarePanelOpen(true);
64+
}
65+
}
66+
67+
return {
68+
isPanelOpen: isPanelOpen,
69+
togglePanel: togglePanel,
70+
};
71+
}
72+
73+
/**
74+
* Get the unique ID for the panel based on the cell ID.
75+
* If the cell ID is not provided, the panel ID is just the table ID.
76+
*/
77+
function getPanelId(id: string, cellId?: CellId | null) {
78+
if (cellId) {
79+
return `${cellId}-${id}`;
80+
}
81+
return id;
82+
}
83+
84+
/**
85+
* Check if the panel is owned by the cell.
86+
*/
87+
function isPanelOwner(panelId: string, cellId?: CellId | null) {
88+
if (cellId) {
89+
return panelId.startsWith(cellId);
90+
}
91+
return false;
92+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/* Copyright 2024 Marimo. All rights reserved. */
2+
3+
import { atom } from "jotai";
4+
5+
/**
6+
* Unique ID of the owner rendering the context-aware panel.
7+
*/
8+
export const contextAwarePanelOwner = atom<string | null>(null);
9+
10+
/**
11+
* If true, the panel is open.
12+
*/
13+
export const contextAwarePanelOpen = atom<boolean>(false);
14+
15+
/**
16+
* If true, the panel is treated as part of the editor.
17+
* When false, the panel overlays the editor content.
18+
*/
19+
export const isPinnedAtom = atom<boolean>(false);
20+
21+
/**
22+
* If true, the panel is cell-aware and will switch content based on the focused cell.
23+
* Else, user needs to manually trigger content switch.
24+
*/
25+
export const isCellAwareAtom = atom<boolean>(false);

0 commit comments

Comments
 (0)