Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ const TableSettings: FC<TableSettingsProps> = ({

const [searchColumn, setSearchColumn] = useState("");
const [indexFocusItem, setIndexFocusItem] = useState(-1);
const [draggingColumn, setDraggingColumn] = useState<string | null>(null);
const [dragOverColumn, setDragOverColumn] = useState<string | null>(null);

const customColumns = useMemo(() => {
return selectedColumns.filter(col => !columns.includes(col));
Expand Down Expand Up @@ -122,6 +124,43 @@ const TableSettings: FC<TableSettingsProps> = ({
onChangeColumns(columnsArray);
}, []);

const handleDragStart = (column: string) => (e: DragEvent) => {
setDraggingColumn(column);
e.dataTransfer?.setData("text/plain", column);
};

const handleDragOver = (column: string) => (e: DragEvent) => {
e.preventDefault();
if (dragOverColumn === column) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dragOverColumn can be stale here. This check can be removed - setting the same value won’t trigger a re-render.

setDragOverColumn(column);
};

const handleDragLeave = () => {
setDragOverColumn(null);
};

const handleDrop = (targetColumn: string) => (e: DragEvent) => {
e.preventDefault();
if (!draggingColumn || draggingColumn === targetColumn) return;

const fromIndex = selectedColumns.indexOf(draggingColumn);
const toIndex = selectedColumns.indexOf(targetColumn);
if (fromIndex === -1 || toIndex === -1) return;

const updatedColumns = [...selectedColumns];
updatedColumns.splice(fromIndex, 1);
updatedColumns.splice(toIndex, 0, draggingColumn);

handleChangeDisplayColumns(updatedColumns);
setDragOverColumn(null);
setDraggingColumn(null);
Comment on lines +155 to +156
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup logic is duplicated here - consider calling handleDragEnd() instead of resetting the state inline:

Suggested change
setDragOverColumn(null);
setDraggingColumn(null);
handleDragEnd();

};

const handleDragEnd = () => {
setDragOverColumn(null);
setDraggingColumn(null);
};

return (
<div className="vm-table-settings">
<Tooltip title={title}>
Expand Down Expand Up @@ -193,6 +232,39 @@ const TableSettings: FC<TableSettingsProps> = ({
))}
</div>
</div>
{!!selectedColumns.length && (
<div className="vm-table-settings-modal-columns-order">
<div className="vm-table-settings-modal-columns-order__title">
Drag to reorder selected columns
</div>
<div className="vm-table-settings-modal-columns-order__list">
{selectedColumns.map((col) => (
<div
className={classNames({
"vm-table-settings-modal-columns-order__item": true,
"vm-table-settings-modal-columns-order__item_dragging": draggingColumn === col,
"vm-table-settings-modal-columns-order__item_over": dragOverColumn === col,
})}
key={col}
draggable
onDragStart={handleDragStart(col)}
onDragOver={handleDragOver(col)}
onDragLeave={handleDragLeave}
onDrop={handleDrop(col)}
onDragEnd={handleDragEnd}
>
<span
className="vm-table-settings-modal-columns-order__drag"
aria-hidden="true"
/>
<span className="vm-table-settings-modal-columns-order__label">
{col}
</span>
</div>
))}
</div>
</div>
)}
</div>
{toggleTableCompact && tableCompact !== undefined && (
<div className="vm-table-settings-modal-section">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,74 @@
}
}
}

&-columns-order {
padding: 0 $padding-global $padding-global;
display: flex;
flex-direction: column;
gap: $padding-small;

&__title {
font-size: $font-size;
font-weight: bold;
}

&__list {
display: flex;
flex-direction: column;
gap: $padding-small;
}

&__item {
display: grid;
grid-template-columns: 16px 1fr;
align-items: center;
gap: $padding-small;
padding: $padding-small $padding-global;
border: $border-divider;
border-radius: $border-radius-small;
background-color: $color-background-block;
cursor: grab;

&_dragging {
cursor: grabbing;
opacity: 0.7;
}

&_over {
background-color: $color-hover-black;
border-color: $color-secondary;
}
}

&__drag {
width: 16px;
height: 16px;
display: inline-flex;
align-items: center;
justify-content: center;
color: $color-text-secondary;

&:before {
content: "";
display: block;
width: 10px;
height: 2px;
background-color: currentColor;
border-radius: 2px;
box-shadow:
0 -4px 0 currentColor,
0 4px 0 currentColor;
}
}

&__label {
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
text-decoration: none;
font-family: $font-family-monospace;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { render, screen } from "@testing-library/preact";
import { describe, expect, it } from "vitest";
import TableLogs from "./TableLogs";

const logs = [
{ _time: "2025-01-01T00:00:00Z", file: "a.go", _msg: "first message" },
{ _time: "2025-01-01T00:01:00Z", file: "b.go", _msg: "second message" },
];

describe("TableLogs", () => {
it("renders columns in the provided display order", () => {
render(
<TableLogs
logs={logs}
displayColumns={["_msg", "file", "_time"]}
tableCompact={false}
columns={["_time", "file", "_msg"]}
rowsPerPage={10}
/>
);

const headers = screen
.getAllByRole("columnheader")
.map((header) => header.textContent?.trim() || "");

expect(headers.slice(0, 3)).toEqual(["_msg", "file", "_time"]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,19 @@ const TableLogs: FC<TableLogsProps> = ({ logs, displayColumns, tableCompact, col
}));
}, [columns]);

const tableColumnsMap = useMemo(() => {
return new Map(tableColumns.map((col) => [String(col.key), col]));
}, [tableColumns]);

const filteredColumns = useMemo(() => {
if (tableCompact) return compactColumns;
if (!displayColumns?.length) return [];
return tableColumns.filter(c => displayColumns.includes(c.key as string));
}, [tableColumns, displayColumns, tableCompact]);
return displayColumns.reduce<typeof tableColumns>((acc, key) => {
const column = tableColumnsMap.get(key);
if (column) acc.push(column);
return acc;
}, []);
}, [tableColumnsMap, displayColumns, tableCompact]);
Comment on lines +55 to +60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the column config is derived from the key, tableColumnsMap/reduce (and even columns as a source list) look redundant here. We can build the Table columns directly from displayColumns to keep the requested order and simplify the code.

Suggested change
return displayColumns.reduce<typeof tableColumns>((acc, key) => {
const column = tableColumnsMap.get(key);
if (column) acc.push(column);
return acc;
}, []);
}, [tableColumnsMap, displayColumns, tableCompact]);
return displayColumns.map((key) => ({
key: key as keyof Logs,
title: key,
className: getColumnClass(key),
}));
}, [displayColumns, tableCompact]);


const paginationOffset = useMemo(() => {
const startIndex = (page - 1) * rowsPerPage;
Expand Down