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

Update dragHandle to display at the intersection of column and row #3355

Merged
merged 11 commits into from
Oct 4, 2023

Conversation

amanmahajan7
Copy link
Contributor

@amanmahajan7 amanmahajan7 commented Oct 2, 2023

image

@amanmahajan7 amanmahajan7 self-assigned this Oct 2, 2023
@amanmahajan7 amanmahajan7 marked this pull request as ready for review October 3, 2023 11:32
@amanmahajan7 amanmahajan7 requested a review from nstepien as a code owner October 3, 2023 11:32
@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Merging #3355 (bc26649) into main (d0c8141) will increase coverage by 0.14%.
The diff coverage is 84.00%.

❗ Current head bc26649 differs from pull request most recent head edfc89d. Consider uploading reports for the commit edfc89d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3355      +/-   ##
==========================================
+ Coverage   82.99%   83.13%   +0.14%     
==========================================
  Files          47       47              
  Lines        5075     5088      +13     
  Branches      788      794       +6     
==========================================
+ Hits         4212     4230      +18     
+ Misses        863      858       -5     
Files Coverage Δ
src/DataGrid.tsx 88.80% <100.00%> (+0.02%) ⬆️
src/DragHandle.tsx 76.66% <80.00%> (+5.23%) ⬆️

src/DataGrid.tsx Outdated
rows={rows}
column={column}
columnWidth={columnWidth}
rowHeight={getRowHeight(rowIdx)}
isLastColumn={column.idx === maxColIdx}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't use idx here?
What if we the cell spans to the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we the cell spans to the end?

place-self fixes it I think. but we still need to handle colSpan on a frozen column 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, may be not. Checking

src/DragHandle.tsx Outdated Show resolved Hide resolved
@amanmahajan7 amanmahajan7 marked this pull request as draft October 3, 2023 13:25
@amanmahajan7 amanmahajan7 marked this pull request as ready for review October 3, 2023 16:12
Comment on lines +136 to +138
insetInlineStart: insetInlineStart
? `calc(${insetInlineStart} + ${columnWidth}px + var(--rdg-drag-handle-size) * -0.5 - 1px)`
: undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It sets the correct left position for the drag handle when it is frozen.

@amanmahajan7 amanmahajan7 merged commit 24a3de8 into main Oct 4, 2023
2 checks passed
@amanmahajan7 amanmahajan7 deleted the am-drag-handle-position branch October 4, 2023 16:55
adityatoshniwal pushed a commit to pgadmin-org/react-data-grid that referenced this pull request Jul 31, 2024
…dazzle#3355)

* Hide drag handle if all the cells in a column are readonly

* Update dragHandle to display at the intersection of column and row

* Fix frozen column

* Cleanup

* `marginInlineStart` is only needed on unfrozen columns

* dedupe

* Use idx

* Use placeself and negative margins

* Remove unused function

* Handle colSpan on the last column
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.

2 participants