Handle collapsed class on triggers even when manually invoked
#14686
Conversation
|
This turned out to be a rather large refactor as virtually all the behavior that happened by clicking a toggle was poorly implemented and did not respect potentially prevented |
|
CC: @fat |
| + '<div class="accordion-group"/>' | ||
| + '<div class="accordion-group"/>' | ||
| + '<div class="accordion-group"/>' | ||
| var accordionHTML = '<div class="panel-group accordion">' |
There was a problem hiding this comment.
perhaps we should make accordion an id instead of a class
There was a problem hiding this comment.
Yeah, that's what I was thinking at first too but the whole unit test is about this group not having an ID but a class.
hnrch02
force-pushed
the
collapsed-class-manual-invocation
branch
from
5916860 to
8c995e1
Compare
| @@ -116,6 +130,33 @@ | |||
| this[this.$element.hasClass('in') ? 'hide' : 'show']() | |||
| } | |||
|
|
|||
| Collapse.prototype.getParent = function () { | |||
There was a problem hiding this comment.
@fat Is this what you meant?
hnrch02
force-pushed
the
collapsed-class-manual-invocation
branch
from
8c995e1 to
4b2646e
Compare
|
Pinging @fat for final review. |
| this.options = $.extend({}, Collapse.DEFAULTS, options) | ||
| this.$trigger = $(this.options.trigger || '[data-toggle="collapse"]').filter('[href="#' + el.id + '"], [data-target="#' + el.id + '"]') |
There was a problem hiding this comment.
if you're worried about this being long, just break it onto multiple lilnes
There was a problem hiding this comment.
why isn't '[data-toggle="collapse"]' just the default value for the trigger option?
|
seems mostly good, just small things |
hnrch02
force-pushed
the
collapsed-class-manual-invocation
branch
from
4b2646e to
ed3a65f
Compare
Handle `collapsed` class on triggers even when manually invoked
|
This seems to still be broken in v3.3.2. Is the expected usage: |
|
@elvirb Please file a new issue with a live example. Thanks. |
Fixes #13636.