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

Use transitionEnd in QUnit since we moved away from PhantomJS #25896

Merged
merged 1 commit into from
Mar 20, 2018

Conversation

Johann-S
Copy link
Member

Remove that code because it's useless since we moved away from PhantomJS, plus by doing that our coverage increased a lot 😍

@Johann-S Johann-S requested a review from XhmikosR March 20, 2018 10:11
@Johann-S Johann-S requested a review from bardiharborow March 20, 2018 10:11
@Johann-S Johann-S force-pushed the v4-johann-qunit-transition branch from 6adaf8a to 51fe56f Compare March 20, 2018 10:12
js/src/util.js Outdated
@@ -14,7 +14,9 @@ const Util = (($) => {
* ------------------------------------------------------------------------
*/

let transition = false
const transition = {
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need it to be an object now?

@Johann-S Johann-S force-pushed the v4-johann-qunit-transition branch from 51fe56f to 6f9c3ff Compare March 20, 2018 10:35
@Johann-S
Copy link
Member Author

Done @XhmikosR, plus I removed Util.supportsTransitionEnd which is useless now 👍

@Johann-S Johann-S force-pushed the v4-johann-qunit-transition branch from 6f9c3ff to fbb3214 Compare March 20, 2018 12:28
@XhmikosR
Copy link
Member

I wonder, won't this break stuff for people using Util.supportsTransitionEnd()?

@Johann-S
Copy link
Member Author

it can be a breaking change but if we let this method it will be the same as :

supportsTransitionEnd() {
  return true
}

because we are compatible with browsers whose support transition end.

@XhmikosR
Copy link
Member

Yeah, I think we should keep compatibility and add a todo comment that this should be removed in v5

@Johann-S Johann-S force-pushed the v4-johann-qunit-transition branch from fbb3214 to f4dbc37 Compare March 20, 2018 14:46
@Johann-S
Copy link
Member Author

I did that 😉 Util.supportsTransitionEnd is back with a TODO

@XhmikosR
Copy link
Member

Thanks!

How about transitionEndTest()? Maybe we should do the same for this too?

@Johann-S
Copy link
Member Author

transitionEndTest isn't callable currently, so it's not a breaking change

@Johann-S Johann-S merged commit bedc96e into v4-dev Mar 20, 2018
@Johann-S Johann-S deleted the v4-johann-qunit-transition branch March 20, 2018 14:56
@mdo mdo mentioned this pull request Mar 20, 2018
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.

2 participants