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

Ensure the drop target remains properly identified for draggable columns #418

Merged

Conversation

RustyToms
Copy link
Contributor

closes #417

Special classes are added to the cells that are being dragged over, so that they can be styled appropriately. However, dragLeave is not just triggered when the drag exits the cell, but also when the drag enters a child element inside the cell.

If the table is implemented with draggable columns, and also with either child elements inside the table header cells, or resizable columns, the is-drag-target class will be removed from the cell even while it is being dragged over. Resizable columns add a extra div (with class lt-column-resizer) inside the table header cell. This means that with both draggable and resizable columns, the drag target styling will rarely be applied when dragging to the left, while it mostly works while dragging to the right.

inconsistent_drag_target

Here is an Ember-twiddle demonstrating the problem: https://ember-twiddle.com/4ab83dd0909fe3bc2083a47c1b0ba6fb?openFiles=components.expandable-table.js%2C

There are a few different ways to approach this problem but there are annoying edge cases that introduce new bugs for a few of them that I tried. This solution is to reset isDragTarget to true in dragOver, since it is constantly being called while the cell is being dragged over. It seems to work pretty well.

consistent_drag_target
Here is a Twiddle demonstrating this solution: https://ember-twiddle.com/e30d04f1e3e1f4cdb3f95749dbeb3f6c?openFiles=components.expandable-table.js%2C

@alexander-alvarez
Copy link
Collaborator

@RustyToms thanks for this. Sorry for the delay.
Do you think the draggable portion of the demo would benefit from having some css associated with these styles as you've demonstrated in your gif?

@offirgolan
Copy link
Collaborator

@alexander-alvarez the demo page already has some (minimal) CSS to show a valid drop target.

Can you verify this work across grouped columns as well?

@Techn1x
Copy link

Techn1x commented Sep 18, 2017

Just encountered this issue today and also made a video.......... but looks like I don't need to post it :)
Glad it's already known about though!

@RustyToms
Copy link
Contributor Author

Hey @offirgolan @alexander-alvarez this is still a pretty bad issue, what do you need right now in order to merge this? Do you still want the demo updated?

@alexander-alvarez
Copy link
Collaborator

@RustyToms so sorry this slipped through the cracks... I checked out your branch it and clearly works as expected, so I'll merge this in.

I don't use the drag/drop functionality, but if you feel that we could do a better job on the documentation side of it, please let us know, or open up a PR.

@alexander-alvarez alexander-alvarez merged commit 2e2faf9 into adopted-ember-addons:master Oct 18, 2017
@RustyToms RustyToms deleted the improve_drag_styles branch October 18, 2017 18:34
buschtoens pushed a commit that referenced this pull request Oct 26, 2017
* feat(custom-icons): base files

* fix(column-base): sort icon displayed with sortable:false

* feat(dummy): cookbook for custom-sort-icon

* loosen ember-one-way-controls version requirement

* yarn.lock updates

* [BUGFIX release]: Added check for target column is droppable (#496)

* [chore]: Update ember-truth-helpers to latest version. (#497)

* [BUGFIX release]: Ensure the drop target remains properly identified (#418)

* fix(feedback): iconComponent comments, iconComponent test

* fix(feedback-v2): iconComponent comments, iconComponent test
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.

Drag target classes are removed when dragging over child elements
4 participants