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

fix: only allow row drag on cell w/dnd or cell-reorder #939

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

ghiscoding
Copy link
Collaborator

@ghiscoding ghiscoding commented Dec 1, 2023

  • a previous PR fix: add allowDragFromClosest to make .slick-cell or child draggable #898 caused a regression on a cell with a select dropdown (like Slick.Editors.YesNoSelect), the regression was caused by the implementation of Draggable allowDragFromClosest which will check if current DOM element is .slick-cell or if not it will also try its parent and that caused the regression because the cell with the editor also had a .slick-cell and so the code taught that the user started a drag and it cancelled event bubbling which in turn also prevented the select dropdown to be clickable.
  • to fix this issue we need to make sure that the cell is queried not just with div.slick-cell but also with certain CSS classes, we need to check if parent has either .dnd or .cell-reorder to permit the dragging when checking parent cell with allowDragFromClosest

- a previous PR #897 caused a regression on a cell with a select dropdown (like `Slick.Editors.YesNoSelect`), the regression was caused by the implementation of Draggable `allowDragFromClosest` which will check if current DOM element is `.slick-cell` or if not it will also try its ancestor and that caused the regression because the cell with the editor also had a `.slick-cell` and so the code taught that the user started a drag and it cancelled event bubbling which in turn also prevented the select dropdown to be clickable.
- to fix this issue we need to make sure that the cell is queried not just with `div.slick-cell` but also with certain CSS classes, we need to check if it has either `.dnd` or `.cell-reorder` to permit the dragging
- scroll stopped working if we ask for dnd/cell-reorder on the slick cell
@ghiscoding ghiscoding merged commit 0f07161 into version4 Dec 1, 2023
2 checks passed
@ghiscoding ghiscoding deleted the bugfix/draggable-regression-v4 branch December 1, 2023 16:06
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