Fix trigger shown and hidden events for dropdown #16865

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@Johann-S
Contributor

Hi,

it's a fix for #16828

Thanks to @DaGLiMiOuX

@twbs-savage
Member

Tests passed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: 98ccf83
Build details: https://travis-ci.org/twbs-savage/bootstrap/builds/72183457

(Please note that this is a fully automated comment.)

@cvrebert cvrebert commented on an outdated diff Jul 24, 2015
js/dropdown.js
@@ -51,7 +51,7 @@
if (e.isDefaultPrevented()) return
$this.attr('aria-expanded', 'false')
- $parent.removeClass('open').trigger('hidden.bs.dropdown', relatedTarget)
+ $parent.removeClass('open').trigger(e = $.Event('hidden.bs.dropdown', relatedTarget))
@cvrebert
cvrebert Jul 24, 2015 Member

The e = is unnecessary.

@cvrebert cvrebert commented on an outdated diff Jul 24, 2015
js/dropdown.js
@@ -85,7 +85,7 @@
$parent
.toggleClass('open')
- .trigger('shown.bs.dropdown', relatedTarget)
+ .trigger(e = $.Event('shown.bs.dropdown', relatedTarget))
@cvrebert
cvrebert Jul 24, 2015 Member

The e = is unnecessary.

@cvrebert cvrebert commented on an outdated diff Jul 24, 2015
js/tests/unit/dropdown.js
+ + '<li><a href="#">Secondary link</a></li>'
+ + '<li><a href="#">Something else here</a></li>'
+ + '<li class="divider"/>'
+ + '<li><a href="#">Another link</a></li>'
+ + '</ul>'
+ + '</li>'
+ + '</ul>'
+ var $dropdown = $(dropdownHTML)
+ .appendTo('#qunit-fixture')
+ .find('[data-toggle="dropdown"]')
+ .bootstrapDropdown()
+ var done = assert.async()
+
+ $dropdown.parent('.dropdown')
+ .on('shown.bs.dropdown', function (e) {
+ assert.notStrictEqual(typeof e, 'undefined');
@cvrebert
cvrebert Jul 24, 2015 Member

Needs to assert that e.relatedTarget equals the correct element

@cvrebert cvrebert commented on an outdated diff Jul 24, 2015
js/tests/unit/dropdown.js
+ + '<li><a href="#">Another link</a></li>'
+ + '</ul>'
+ + '</li>'
+ + '</ul>'
+ var $dropdown = $(dropdownHTML)
+ .appendTo('#qunit-fixture')
+ .find('[data-toggle="dropdown"]')
+ .bootstrapDropdown()
+ var done = assert.async()
+
+ $dropdown.parent('.dropdown')
+ .on('shown.bs.dropdown', function (e) {
+ assert.notStrictEqual(typeof e, 'undefined');
+ })
+ .on('hidden.bs.dropdown', function (e) {
+ assert.notStrictEqual(typeof e, 'undefined')
@cvrebert
cvrebert Jul 24, 2015 Member

Needs to assert that e.relatedTarget equals the correct element

@cvrebert cvrebert added the js label Jul 24, 2015
@Johann-S Johann-S Fix trigger shown and hidden events for dropdown
6aafcfb
@twbs-savage
Member

Tests passed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: 6aafcfb
Build details: https://travis-ci.org/twbs-savage/bootstrap/builds/72423124

(Please note that this is a fully automated comment.)

@Johann-S
Contributor

Thanks @cvrebert for your feedbacks :shipit:

@cvrebert cvrebert added this to the v3.3.6 milestone Jul 27, 2015
@cvrebert cvrebert added a commit that closed this pull request Jul 27, 2015
@Johann-S @cvrebert Johann-S + cvrebert Fix triggering of {shown,hidden}.bs.dropdown events so relatedTarget …
…gets set properly

Fixes #16828
Closes #16865
ef1ce9a
@cvrebert cvrebert closed this in ef1ce9a Jul 27, 2015
@mdo mdo referenced this pull request Jul 27, 2015
Closed

v3.3.6 ship list #16644

@cvrebert
Member

Thanks!

@kkirsche kkirsche referenced this pull request in elastic/kibana Feb 21, 2016
Merged

Update Bootstrap to 3.3.6 #6294

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment