v4 Dropdown without jQuery#24099
Conversation
Johann-S
force-pushed
the
v4-dropdown-without-jquery
branch
from
7c00ab6 to
dab2c3a
Compare
Johann-S
force-pushed
the
v4-without-jquery
branch
from
76ee411 to
38bebc9
Compare
Johann-S
force-pushed
the
v4-dropdown-without-jquery
branch
from
dab2c3a to
59b5f85
Compare
Johann-S
force-pushed
the
v4-without-jquery
branch
from
38bebc9 to
e692aa3
Compare
Johann-S
force-pushed
the
v4-dropdown-without-jquery
branch
from
59b5f85 to
b5f26c9
Compare
Johann-S
force-pushed
the
v4-without-jquery
branch
from
e692aa3 to
ab6c322
Compare
| e.stopPropagation() | ||
| }) | ||
| EventHandler.on(document, Event.KEYDOWN_DATA_API, Selector.DATA_TOGGLE, Dropdown._dataApiKeydownHandler) | ||
| EventHandler.on(document, Event.KEYDOWN_DATA_API, Selector.MENU, Dropdown._dataApiKeydownHandler) |
There was a problem hiding this comment.
@alekitto they are an issue here with our current EventHandler because if we have the same delegation handler for a different delegation selector no event listener is added due to that line : https://github.com/twbs/bootstrap/blob/v4-without-jquery/js/src/dom/eventHandler.js#L225
There was a problem hiding this comment.
Fixed by my last commit 👍
There was a problem hiding this comment.
Yes, you're right, I managed to reproduce the error in a test case. PR is coming to fix this.
Johann-S
force-pushed
the
v4-dropdown-without-jquery
branch
from
b5f26c9 to
f3e6cd8
Compare
| } else { | ||
| addHandler(element, event, complete) | ||
| } | ||
| realHandler.uid = complete.uid.toString() |
There was a problem hiding this comment.
@Johann-S Adding the uid property to the real handler function will cause troubles when using and removing the same handler for more than one event (like in https://github.com/twbs/bootstrap/blob/v4-without-jquery/js/src/tooltip.js#L322).
It was changed in fa90114 to take this into account, removing the setting of uidEvent property.
I thought that a test was written for this scenario, but i've probably missed it.
There was a problem hiding this comment.
Yep that's because Util.noop always return the same function instead of a new one here but yes in case of a reusable handler it will be problematic
There was a problem hiding this comment.
Yes, with Util.noop shouldn't be a problem 😄
However, #24487 should fix that, but i think you have to revert your last commit in order to merge it. If you can reopen it, I'll submit a couple of tests to cover this case.
Johann-S
force-pushed
the
v4-dropdown-without-jquery
branch
2 times, most recently
from
15ca804 to
ff11028
Compare
|
I'm stuck on this unit test : https://github.com/twbs/bootstrap/blob/v4-dropdown-without-jquery/js/tests/unit/dropdown.js#L598 It seems I don't understand how but when I trigger a click event on the input, the event is swallowed 🤔 |
Johann-S
force-pushed
the
v4-without-jquery
branch
from
e310e55 to
d252d27
Compare
Johann-S
force-pushed
the
v4-dropdown-without-jquery
branch
from
ff11028 to
93e8856
Compare
2 participants
Work in progress...