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

remove hardcoded timeout by listening on transitions #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ianstormtaylor
Copy link
Contributor

really cool component by @anthonyshort that lets us remove hardcoded timeouts for waiting for transitions, looking to use overlay with a .3s transition instead and figured this would make a good fix

@luka5
Copy link

luka5 commented Jul 4, 2013

this will probably fix issue #7

@ianstormtaylor
Copy link
Contributor Author

shit, this doesn't fix it. unfortunately it seems like there's no way to do this without knowing the element is transitioning, so this is just currently impossible because the spec is poorly done.

@anthonyshort
Copy link

Flip the addClass and after calls and it should work. I've been using that component for a while on a bunch of things.

@ianstormtaylor
Copy link
Contributor Author

mm definitely dumb on my part for having that before hand. seems to not work with version 0.2.0 of https://github.com/anthonyshort/has-transitions though since now the el itself is being checked?

if i use https://github.com/anthonyshort/after-transition 0.0.2 it works, but 0.0.3 doesnt

might be doing something else dumb, let me know. trouble again (i think) is that the animation is pure css, no inline styles

@anthonyshort
Copy link

I usually just toggle classes on and off and it works ok. The change I made to that component in those versions (I think) was changed from checking if the browser is capable of doing to transitions, to actually checking the el.style.transition property to see if the element has a transition set on it. There's usually no point in doing the afterTransition if the element doesn't actually have a transition property.

Soooo by doing the afterTransition and THEN doing the class, it will check the style, see there are no transition properties, and then just execute it straight away.

I think I just need to figure out which is better for the component - check the capabilities (even if the element has no transitions) or check the element for transitions.

@anthonyshort
Copy link

Actually, looking at how I'm checking it, it's only checking the el.style.transition, so if you're not using the shorthand and using transition-property, transition-duration etc, that might return false.

I might just make afterTransition assume you know what you're doing and just check for browser support for transitions.

@pirxpilot
Copy link
Member

I pushed rebased and updated version of this to transition-end branch - and it seems to work for me, but...

  • would be great if people checked on older browsers - I am happily Windows and IE free at the moment
  • isn't it an overkill? I mean after-transition is great and useful component, but maybe here we can make older browsers' users happy by waiting 300ms instead of 2s

pirxpilot added a commit that referenced this pull request Apr 25, 2014
When hiding overlay we keep it in the DOM tree for a short amount of
time to make sure that CSS transition is visible. For most browsers it
does not matter how long is that period, because our CSS prevents hidden
overlay from capturing any pointer events.

Since IE < 11 does not support `pointer-events` property for CSS[1],
hiding overlay keeps browser unresponsive for ~2s.

This commit shortens that period to 350ms, which should allow transition
to complete, and make IE usable a bit faster.

fixes #7
see #13 for a fix that removes hardcoded timeout

[1]: http://caniuse.com/#search=pointer-events
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.

4 participants