Collapse with flex elements #22758

Johann-S · 2017-06-06T14:53:27Z

Detect if the element use flexbox, if it's the case add .flexshow

Add Visual test
Detect all posible flex values (inline-flex, flex, etc...)

We 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 update method if the collapsible element change, to detect once again the appropriate show class

Close : #22600

/CC : @mdo

glebm · 2017-06-07T01:43:05Z

js/src/collapse.js

+      // Detect flex for inline style
+      let useFlex = $(this._element).css('display').indexOf('flex') !== -1
+
+      // Create a wrapper to hide our flex detection


Can you check getComputedStyle(this._element).display instead?

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

mdo · 2017-06-15T03:57:39Z

Is anything here considered to be a breaking change @Johann-S? Might be beneficial to wait for Beta 2 to address this.

Johann-S · 2017-06-15T04:11:24Z

That's right this can wait for beta 2

XhmikosR · 2017-08-13T12:10:38Z

js/src/collapse.js

+      const tabClass = this._element.classList
+
+      // Detect flex for inline style
+      let useFlex = $(this._element).css('display').indexOf('flex') !== -1


How about just using https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/includes?

I switched to includes but they are no support on IE 10 so back to indexOf

But it's transpiled isn't it?

I didn't checked but unit tests failed

But since we use babel it should be taken care of. Please try one more time.

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.

Do you know which polyfill to use ? I can make a test 👍

https://babeljs.io/docs/usage/polyfill/

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

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.

XhmikosR · 2017-08-13T12:11:15Z

js/tests/visual/collapse.html

    </div>

-    <script src="../../../docs/assets/js/vendor/jquery-slim.min.js"></script>
+    <script src="../../../assets/js/vendor/jquery-slim.min.js"></script>


Hmm, how did it work without this?

It didn't work before that change and yes it's an unrelated change

XhmikosR · 2017-08-13T12:14:30Z

docs/4.0/components/collapse.md

@@ -226,6 +226,10 @@ Shows a collapsible element. **Returns to the caller before the collapsible elem

 Hides a collapsible element. **Returns to the caller before the collapsible element has actually been hidden** (i.e. before the `hidden.bs.collapse` event occurs).

+#### `.collapse('update')`
+
+Update a collapsible element if this element changed during his lifetime


its lifetime

And maybe add a . at the end.

XhmikosR · 2017-08-17T17:45:18Z

docs/4.0/components/collapse.md

@@ -226,6 +226,10 @@ Shows a collapsible element. **Returns to the caller before the collapsible elem

 Hides a collapsible element. **Returns to the caller before the collapsible element has actually been hidden** (i.e. before the `hidden.bs.collapse` event occurs).

+#### `.collapse('update')`
+
+Update a collapsible element if this element changed during its lifetime.


How about: "Update a collapsible element if it changed during its lifetime."

Yep that's better thanks 👍 I'll update that soon

Johann-S · 2017-08-23T08:26:33Z

For me everything is fine @mdo so this one can be merged 👍

vanillajonathan · 2017-11-09T09:24:12Z

Keep in mind that the other .flex-* classes are separated with a hyphen.
Example .flow-row, .flex-column, .flex-wrap, etc.'.

Perhaps .flex-show should be considered.

vanillajonathan · 2017-11-09T09:25:14Z

scss/_transitions.scss

 }

 tr {
-  &.collapse.show {
+  &.collapse.show,
+  &.collapse.flexshow {


Should .flexshow be named .flex-show to be consistent with the hyphen separation used by the other .flex-* classes?

Johann-S · 2018-01-18T08:08:27Z

Not sure to keep that PR, because I have to create an hidden div to detect if folks use flex attributes or not, a better solution would be CSS one

Johann-S added css js v4 labels Jun 6, 2017

Johann-S force-pushed the v4-collapse-flex branch from dd51141 to 598db14 Compare June 6, 2017 20:42

glebm reviewed Jun 7, 2017

View reviewed changes

Johann-S added this to the v4.0.0-beta milestone Jun 7, 2017

Johann-S removed this from the v4.0.0-beta milestone Jun 15, 2017

XhmikosR reviewed Aug 13, 2017

View reviewed changes

Johann-S force-pushed the v4-collapse-flex branch 2 times, most recently from 41bdf59 to 190a76f Compare August 17, 2017 17:22

XhmikosR reviewed Aug 17, 2017

View reviewed changes

Johann-S force-pushed the v4-collapse-flex branch from 190a76f to 7ad5ea1 Compare August 19, 2017 15:59

Johann-S force-pushed the v4-collapse-flex branch from 7ad5ea1 to 1768199 Compare August 23, 2017 08:28

Johann-S force-pushed the v4-collapse-flex branch from 1768199 to 14c347c Compare September 14, 2017 08:25

Johann-S force-pushed the v4-collapse-flex branch from 14c347c to bbb658d Compare October 3, 2017 08:09

tmorehouse mentioned this pull request Oct 4, 2017

[Informational] Bootstrap V4.beta 1, 2 & 3 ship list bootstrap-vue/bootstrap-vue#747

Closed

56 tasks

Johann-S added 2 commits October 20, 2017 10:01

Collapse with flex elements

df1f914

Add visual test

d15f2cf

Add update method for Collapse plugin

05c311c

Johann-S force-pushed the v4-collapse-flex branch from bbb658d to 05c311c Compare October 20, 2017 08:02

vanillajonathan reviewed Nov 9, 2017

View reviewed changes

MartijnCuppens mentioned this pull request Mar 7, 2018

Collapse with flex elements - simple approach #25806

Merged

Johann-S closed this Mar 8, 2018

Johann-S deleted the v4-collapse-flex branch March 8, 2018 08:13

SkuterPL mentioned this pull request May 23, 2018

.nav .flex-column and collapse - problem with display block #26567

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collapse with flex elements #22758

Collapse with flex elements #22758

Johann-S commented Jun 6, 2017 •

edited

Loading

glebm Jun 7, 2017 •

edited

Loading

Johann-S Jun 7, 2017

mdo commented Jun 15, 2017

Johann-S commented Jun 15, 2017

XhmikosR Aug 13, 2017

Johann-S Aug 17, 2017

XhmikosR Aug 17, 2017

Johann-S Aug 17, 2017

XhmikosR Aug 17, 2017

XhmikosR Aug 19, 2017

Johann-S Aug 19, 2017

XhmikosR Aug 19, 2017

Johann-S Sep 14, 2017

XhmikosR Sep 14, 2017

XhmikosR Aug 13, 2017

Johann-S Aug 17, 2017

XhmikosR Aug 13, 2017

XhmikosR Aug 13, 2017

XhmikosR Aug 17, 2017

Johann-S Aug 17, 2017

Johann-S commented Aug 23, 2017

vanillajonathan commented Nov 9, 2017

vanillajonathan Nov 9, 2017

Johann-S commented Jan 18, 2018

Collapse with flex elements #22758

Collapse with flex elements #22758

Conversation

Johann-S commented Jun 6, 2017 • edited Loading

glebm Jun 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdo commented Jun 15, 2017

Johann-S commented Jun 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Johann-S commented Aug 23, 2017

vanillajonathan commented Nov 9, 2017

Choose a reason for hiding this comment

Johann-S commented Jan 18, 2018

Johann-S commented Jun 6, 2017 •

edited

Loading

glebm Jun 7, 2017 •

edited

Loading