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

add special transitionend type to test event origin #13786

Merged
merged 1 commit into from
Jun 12, 2014
Merged

add special transitionend type to test event origin #13786

merged 1 commit into from
Jun 12, 2014

Conversation

fat
Copy link
Member

@fat fat commented Jun 11, 2014

fixes #13430


$.event.special['bs-transitionend'] = {
bindType: $.support.transition.end,
delegateType: $.support.transition.end,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be more like ($.support.transition && $.support.transition.end) || false for it to be null safe? Also, does jQuery care if these are false?

Edit: Or we could wrap this in a conditional to check if transitions are supported

@hnrch02
Copy link
Collaborator

hnrch02 commented Jun 11, 2014

Did you discard the idea of it also handling the emulateTransitionEnd part?

@hnrch02
Copy link
Collaborator

hnrch02 commented Jun 11, 2014

Also, I dislike the dash, but camel-casing events is also weird... Never mind then.

@fat
Copy link
Member Author

fat commented Jun 11, 2014

Did you discard the idea of it also handling the emulateTransitionEnd part?

Yeah, i decided it was probably best not to throw everything into a single function - thought your original proposal was more elegant/flexible.

Also, I dislike the dash, but camel-casing events is also weird... Never mind then.

Yeah not super crazy about the dash either but wanted to namespace somehow… actually i think camel-casing is a better idea. More inline with vender prefix's like webkitTransitionEnd, etc


if (!$.support.transition.end) return

$.event.special['bsTransitionEnd'] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use dot notation then.

@fat
Copy link
Member Author

fat commented Jun 11, 2014

bsTransitionEnd or bsTransitionend ? can't decide

@@ -48,6 +48,16 @@

$(function () {
$.support.transition = transitionEnd()

if (!$.support.transition.end) return
Copy link
Collaborator

Choose a reason for hiding this comment

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

Guard ALL the things
(Excuse my spam)

Copy link
Member Author

Choose a reason for hiding this comment

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

ha - no it was good, made me find a bug

@hnrch02
Copy link
Collaborator

hnrch02 commented Jun 11, 2014

bsTransitionEnd is more in line with the browser vendors, like the example of webkit you posted.

@fat
Copy link
Member Author

fat commented Jun 11, 2014

word, that's what i was thinking

@fat
Copy link
Member Author

fat commented Jun 11, 2014

lg?

@cvrebert cvrebert added the js label Jun 11, 2014
$(this).one($.support.transition.end, function () { called = true })
var callback = function () { if (!called) $($el).trigger($.support.transition.end) }
$(this).one('bsTransitionEnd', function () { called = true })
var callback = function () { if (!called) $($el).trigger(`.end) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happened here?

@cvrebert
Copy link
Collaborator

Add a unit test perhaps?

@fat
Copy link
Member Author

fat commented Jun 11, 2014

ya good call

@fat
Copy link
Member Author

fat commented Jun 11, 2014

erm… can't add unit test because we set transitions to false in unit tests

@cvrebert cvrebert added this to the v3.2.0 milestone Jun 11, 2014
@hnrch02
Copy link
Collaborator

hnrch02 commented Jun 11, 2014

We could set it to transitionend for this test and verify if the origin check part works, no?

@fat
Copy link
Member Author

fat commented Jun 12, 2014

$.support.transition = false is set in the unit html at a global level.

Because transitionEnd is private to the transition method, we have no way to determine the transitionend and force the bsTransitionEnd event in an isolated test.

i'm going to merge this, if you want to hack up a way to unit test it though, that would be rad… just a bit of a pita

fat added a commit that referenced this pull request Jun 12, 2014
add special transitionend type to test event origin
@fat fat merged commit 696632d into master Jun 12, 2014
@fat fat deleted the fat-13430 branch June 12, 2014 05:44
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.

3 participants