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 bug: onDragLeave doesn't reset over-class #643

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

denkan
Copy link
Contributor

@denkan denkan commented Jun 13, 2016

Fixed bug causing nv-file-over (over-class) not resetting when leaving element.
Issue e.g. #514 and #231.

Fixed by removing the check (and cancel) if the event.currentTarget is same as the nv-file-over element. Seems like we don't need it because the onDragOver event will put it right back anyway.

Tested in Chrome, Firefox, Safari.
Could need testing in IE?

@nervgh
Copy link
Owner

nervgh commented Jun 14, 2016

Thanks, but what about these issues?

#332
#237
#450
#312
#331
#286

@denkan
Copy link
Contributor Author

denkan commented Jun 14, 2016

Yea, sorry... should've known it wasn't that simple :)
I'll be happy to take a look when I have little more time.

Great module btw!

@denkan
Copy link
Contributor Author

denkan commented Jun 14, 2016

Alright, I couldn't really put it out of my mind.
I got it working fine on my test browsers by wrapping the onDragLeave logic into a timer @ 50ms. By doing that, the onDragOver has time to reset the timer if it's still hovering/dragging valid element.

A bit hacky but seems to work...

@theoomoregbee
Copy link
Collaborator

I just bower installed ur directive same issue , the over-class still persist after on drag out

@theoomoregbee
Copy link
Collaborator

theoomoregbee commented Aug 5, 2016

This is what i finally used in manipulating it from my controller

var element=angular.element(document.getElementById("basic"));
            element.on('dragleave', function () {
                        element.removeClass("nv-file-over");
                        element.removeClass("test"); //used for my over-class
                        console.log('dragleave');
                    });
`

@denkan
Copy link
Contributor Author

denkan commented Aug 9, 2016

@Theo4u: you installed from bower, meaning you didn't try my fix in this pull request?

@nervgh: have you had the time to look into my last suggestion, using timeout? Any thoughts?

@theoomoregbee
Copy link
Collaborator

the request should have been merged already, am working on a project here , so i dont really have time to check, it should be updated for the next version , so bower install should give the latest updates. Thanks

@nervgh
Copy link
Owner

nervgh commented Aug 14, 2016

@nervgh: have you had the time to look into my last suggestion, using timeout? Any thoughts?

It looks like your code will cause the flicker effect again.

@denkan
Copy link
Contributor Author

denkan commented Aug 21, 2016

@nervgh: Really? That's odd, it's working fine on my side.

Just to be sure; I'm aware that my initial PR commit caused the flicker effect again, but you did see my later commit (226e6e6) where I use the timeout to fix that? Still causing the flickering on your end?

@Stalinko
Copy link

Stalinko commented Jan 8, 2018

This fix is the best one and fully solves the issue. Unlike #720.

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

Successfully merging this pull request may close these issues.

4 participants