Conversation
dd51141 to
598db14
Compare
| // Detect flex for inline style | ||
| let useFlex = $(this._element).css('display').indexOf('flex') !== -1 | ||
|
|
||
| // Create a wrapper to hide our flex detection |
There was a problem hiding this comment.
Can you check getComputedStyle(this._element).display instead?
There was a problem hiding this comment.
I can use that for the line 316 yes, but not for the detection below because getComputedStyle return the current applied style and that's not what I want here.
I want to know if they are a class which contain a flex display
|
Is anything here considered to be a breaking change @Johann-S? Might be beneficial to wait for Beta 2 to address this. |
|
That's right this can wait for beta 2 |
| const tabClass = this._element.classList | ||
|
|
||
| // Detect flex for inline style | ||
| let useFlex = $(this._element).css('display').indexOf('flex') !== -1 |
There was a problem hiding this comment.
There was a problem hiding this comment.
I switched to includes but they are no support on IE 10 so back to indexOf
There was a problem hiding this comment.
I didn't checked but unit tests failed
There was a problem hiding this comment.
But since we use babel it should be taken care of. Please try one more time.
There was a problem hiding this comment.
Just keep in mind we can do this if we just load the babel polyfill and use ES6 methods. Not sure what the exact size diff is, just thinking out loud for another PR.
There was a problem hiding this comment.
Do you know which polyfill to use ? I can make a test 👍
There was a problem hiding this comment.
There was a problem hiding this comment.
I checked babel-polyfill and currently when we import babel-polyfill we will import all the available polyfill not the includes one so for now we should merge this PR and open an issue on how to integrate babel-polyfill with only specific es6 polyfill
There was a problem hiding this comment.
No worries, it's not so important currently as long as we don't use features that aren't available for the browsers we support.
| </div> | ||
|
|
||
| <script src="../../../docs/assets/js/vendor/jquery-slim.min.js"></script> | ||
| <script src="../../../assets/js/vendor/jquery-slim.min.js"></script> |
There was a problem hiding this comment.
Hmm, how did it work without this?
There was a problem hiding this comment.
It didn't work before that change and yes it's an unrelated change
docs/4.0/components/collapse.md
Outdated
|
|
||
| #### `.collapse('update')` | ||
|
|
||
| Update a collapsible element if this element changed during his lifetime |
41bdf59 to
190a76f
Compare
docs/4.0/components/collapse.md
Outdated
|
|
||
| #### `.collapse('update')` | ||
|
|
||
| Update a collapsible element if this element changed during its lifetime. |
There was a problem hiding this comment.
How about: "Update a collapsible element if it changed during its lifetime."
There was a problem hiding this comment.
Yep that's better thanks 👍 I'll update that soon
190a76f to
7ad5ea1
Compare
|
For me everything is fine @mdo so this one can be merged 👍 |
7ad5ea1 to
1768199
Compare
1768199 to
14c347c
Compare
14c347c to
bbb658d
Compare
bbb658d to
05c311c
Compare
|
Keep in mind that the other Perhaps |
| tr { | ||
| &.collapse.show { | ||
| &.collapse.show, | ||
| &.collapse.flexshow { |
There was a problem hiding this comment.
Should .flexshow be named .flex-show to be consistent with the hyphen separation used by the other .flex-* classes?
|
Not sure to keep that PR, because I have to create an hidden div to detect if folks use |
Detect if the element use flexbox, if it's the case add
.flexshowWe detect flex by creating hidden div(s) and we add each class to this hidden div to detect if this div is displayed in flex mode or not.
We do that only when we call the constructor of Collapse plugins that's why I added an
updatemethod if the collapsible element change, to detect once again the appropriate show classClose : #22600
/CC : @mdo