Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Drag & Drop fast follow for card style rows #991

Merged
merged 10 commits into from
Jan 16, 2024
Merged
45 changes: 45 additions & 0 deletions src/components/Table/GridTable.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2020,3 +2020,48 @@ export function DraggableNestedRows() {
/>
);
}

export function DraggableCardRows() {
const dragColumn = dragHandleColumn<Row>({});
const nameColumn: GridColumn<Row> = {
header: "Name",
data: ({ name }) => ({ content: <div>{name}</div>, sortValue: name }),
};

const actionColumn: GridColumn<Row> = {
header: "Action",
data: () => <div>Actions</div>,
clientSideSort: false,
};

let rowArray: GridDataRow<Row>[] = new Array(26).fill(0);
rowArray = rowArray.map((elem, idx) => ({
kind: "data",
id: "" + (idx + 1),
order: idx + 1,
data: { name: "" + (idx + 1), value: idx + 1 },
draggable: true,
}));

const [rows, setRows] = useState<GridDataRow<Row>[]>([simpleHeader, ...rowArray]);

// also works with as="table" and as="virtual"
return (
<GridTable
columns={[dragColumn, nameColumn, actionColumn]}
onRowDrop={(draggedRow, droppedRow, indexOffset) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

question: I'm a little late to the game here, but wouldn't draggedRow and droppedRow be the same row?
It is probably too late to go back and update the implementation at this point, but just spit-balling here; Could we have gotten away with a callback like:

onRowDrop: (row: GridDataRow, newIndex: number) => void

This would give the user the information that a certain row was moved to newIndex.

Or many even:

onRowDrop: (newRows: GridDataRow[]) => void

If the developer only ever needs to just update their row property, this might make their implementations simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

draggedRow would be the row that was dragged initially, droppedRow would be the row we just dropped the draggedRow onto. We could potentially simplify this callback if we only want it to support reordering, but potentially it could support other kinds of drag & drop behavior and in those cases we want the handler to have info about both rows.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, yes. That is a bit confusing of a name then. I would've assume the row being "dropped" is the row I was dragging. Maybe targetRow is a better name?
Interesting point about other drag and drop behaviors. Are we being asked to support dropping into another row in order to make the draggedRow a child of the target/droppedRow?
I'd be tempted to support multiple callbacks for that... like onReorder for the current use case, and .... I dunno what else? Would we want to allow rows to drop into each other to nest them or something? I'd be tempted to solution only the problem in front of us instead of envision future scenarios.
I'm fine with leaving it as is for now.

const tempRows = [...rows];
// remove dragged row
const draggedRowIndex = tempRows.findIndex((r) => r.id === draggedRow.id);
const reorderRow = tempRows.splice(draggedRowIndex, 1)[0];

const droppedRowIndex = tempRows.findIndex((r) => r.id === droppedRow.id);

// insert it at the dropped row index
setRows([...insertAtIndex(tempRows, reorderRow, droppedRowIndex + indexOffset)]);
}}
rows={[...rows]}
style={cardStyle}
/>
);
}
Loading
Loading