Skip to content

Drag Image Click Anchor #939

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kcvikander
Copy link

@kcvikander kcvikander commented Apr 8, 2024

When using the built in drag and drop functionality by setting isDraggable to true, the dragImage is automatically anchored to the center of the drag image.

This update adds a new prop, dragImageAnchor to allow changing where the cursor is anchored to on the drag image.

@kcvikander kcvikander marked this pull request as draft April 8, 2024 17:47
@kcvikander kcvikander changed the title Drag Anchor Update Drag Image Click Anchor Apr 8, 2024
@kcvikander kcvikander marked this pull request as ready for review April 8, 2024 18:40
@lukasmasuch lukasmasuch requested a review from Copilot June 16, 2025 22:51
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new dragImageAnchor prop to control where the cursor attaches to the drag preview when isDraggable is enabled. It adds the prop to the public API, wires it through all relevant grid components, implements the offset logic in the drag start handler, and provides a Storybook example.

  • Define dragImageAnchor in DataGridProps with default "center".
  • Compute custom anchor offsets (dragAnchorX/Y) in the drag-start handler.
  • Drill the prop through DataGrid, DataGridSearch, DataGridDnd, ScrollingDataGrid, and DataEditor, plus a new story.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/core/src/internal/scrolling-data-grid/scrolling-data-grid.tsx Drill dragImageAnchor into GridScroller
packages/core/src/internal/data-grid/data-grid.tsx Add dragImageAnchor prop, default value, and compute offsets in drag start
packages/core/src/internal/data-grid-search/data-grid-search.tsx Pass dragImageAnchor to search-enabled grid
packages/core/src/internal/data-grid-dnd/data-grid-dnd.tsx Pass dragImageAnchor to DnD-enabled grid
packages/core/src/data-editor/stories/data-editor.stories.tsx New DraggableWithEventAnchor story
packages/core/src/data-editor/data-editor.tsx Drill dragImageAnchor through DataEditor impl
Comments suppressed due to low confidence (2)

packages/core/src/internal/data-grid/data-grid.tsx:1577

  • Consider adding unit or integration tests to verify each of the dragImageAnchor modes ("center", "click", "click-x", "click-y") and ensure the computed anchor offsets behave as expected.
const dragAnchorX = ['click', 'click-x'].includes(dragImageAnchor) ? event.clientX - boundsForDragTarget.x : boundsForDragTarget.width / 2

packages/core/src/internal/data-grid/data-grid.tsx:193

  • [nitpick] The indentation of the JSDoc block for dragImageAnchor doesn’t match surrounding props; aligning it with the rest of DataGridProps will keep the style consistent.
    /**

Comment on lines +1577 to +1578
const dragAnchorX = ['click', 'click-x'].includes(dragImageAnchor) ? event.clientX - boundsForDragTarget.x : boundsForDragTarget.width / 2
const dragAnchorY = ['click', 'click-y'].includes(dragImageAnchor) ? event.clientY - boundsForDragTarget.y : boundsForDragTarget.height / 2
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

[nitpick] The logic for computing dragAnchorX and dragAnchorY is nearly identical; extracting it into a small helper (e.g., computeDragAnchor(offset, size, mode)) could improve readability and reuse.

Suggested change
const dragAnchorX = ['click', 'click-x'].includes(dragImageAnchor) ? event.clientX - boundsForDragTarget.x : boundsForDragTarget.width / 2
const dragAnchorY = ['click', 'click-y'].includes(dragImageAnchor) ? event.clientY - boundsForDragTarget.y : boundsForDragTarget.height / 2
const dragAnchorX = computeDragAnchor(event.clientX, boundsForDragTarget.width, dragImageAnchor, boundsForDragTarget.x);
const dragAnchorY = computeDragAnchor(event.clientY, boundsForDragTarget.height, dragImageAnchor, boundsForDragTarget.y);

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant