tabs: fix nested tabs.

[XhmikosR]

When one uses say a carousel inside a tab, the .active selector previously matches the carousel ones too leading to broken tabs.

It's not the perfect solution but should the job for now.

/CC @Johann-S

PS. The data-target solution you suggested seems definitely better for the long term. I just don't have the time to implement it myself so I resorted to a quick fix for now :)

Fixes #22654.

[XhmikosR]

...and tests fail :D

[Johann-S]

Test failed because with this change this line :

previous = $.makeArray($(listElement).find(Selector.ACTIVE))

Retrieve nothing, so we are unable to find previous active items

[XhmikosR]

I should probably also add a test for my use case that currently fails.

@Johann-S: feel free to push to this branch if you have any ideas.

[Johann-S]

LGTM 👍

Unless @XhmikosR want to add another unit test here

[mdo]

Slating this for Beta 2. Thanks for tackling it!

[XhmikosR]

@Johann-S: I just tested this and indeed fixes the issue for my use case. Not sure if I should add my test case though.

@mdo: this is a regression I believe. So it shouldn't be postponed.

[mdo]

@mdo: this is a regression I believe. So it shouldn't be postponed.

We're going to have things we've broken in the beta—we won't have a perfect ship—and this isn't critical IMO. The remaining time ahead of Beta 1 should be spent on ensuring nothing that is current broken (or working hah) needs a backwards incompatible change.

[danijelGombac]

This fix is also not good for my nested tabs. I stick with

ACTIVE_CHILD : '> .nav-item > .active, > .active',

from alpha 6 tabs.

[Johann-S]

I'll check that thank you @danijelGombac for the feedback 👍

[danijelGombac]

I made different tabs with collapse inside. Maybe i should make different codepen.

[Johann-S]

It seems it works pretty well here : https://codepen.io/Johann-S/pen/zzQEje
with my changes

[danijelGombac]

Later i make a codepen, so you can see what is wrong and if tabs will support this type of approach.

[Johann-S]

Yes do not hesitate to provide a reduce test case @danijelGombac 👍

[danijelGombac]

Tabs with collapse. Look Product discounts tab. There is also problem with bg colors.

https://codepen.io/danijelGombac/pen/yXWKdL

[Johann-S]

I made a fix for your issue @danijelGombac but I didn't handled the problem with background colors

[danijelGombac]

Ok, i will test it. The background color problem lies in this part of code:

.nav-pills {
  .nav-link {
    @include border-radius($nav-pills-border-radius);

    &.active,
    .show > & {
      color: $nav-pills-link-active-color;
      background-color: $nav-pills-link-active-bg;
    }
  }
}

Come out:

.nav-pills .nav-link.active,
.show > .nav-pills .nav-link {
  color: #fff;
  background-color: #007bff;
}

And when collapse became collapse.show change all .nav-links.

[Johann-S]

Maybe this CSS issue should be solved in an other PR

[danijelGombac]

@Johann-S

I try your code, but still something not working well. You can see in the last two tabs. When you click the last two tabs and then the tab with collapse, they remain active and from the tabs in collapse the active state is removed.

https://codepen.io/danijelGombac/pen/yXWKdL

[Johann-S]

I made a new commit @danijelGombac which should fix your issue 😉

[danijelGombac]

I try the code and seems to work fine. 👍

[Johann-S]

I made a bit of refactorization here :

      let activeElements
      if (callback === undefined) {
        if (container.nodeName === 'UL') {
          activeElements = $(container).find(Selector.ACTIVE_UL)
        } else {
          activeElements = $(container).find(Selector.ACTIVE)
        }
      } else {
        activeElements = $(container).children(Selector.ACTIVE)
      }

changed to :

      let activeElements
      if (container.nodeName === 'UL') {
        activeElements = $(container).find(Selector.ACTIVE_UL)
      } else {
        activeElements = $(container).children(Selector.ACTIVE)
      }

I played every CodePen submited by @danijelGombac and everythings works fine, Unit tests passed, if you have time @XhmikosR to check on your personal use case too it would be great 👍
I promise it's the last time I change this code (on this PR 😆)

[XhmikosR]

@Johann-S: there's a regression somewhere again, I'll ping you on Slack.

[Johann-S]

@mdo we have a question (@XhmikosR and myself) about our Tabs plugins, we aren't sure if our Tabs plugin should handle a markup like this with the .active class on the li element instead of the a element :

<ul class="nav nav-tabs nav-justified" role="tablist">
  <li class="nav-item">
    <a href="#home" class="nav-link" role="tab" data-toggle="tab">Home</a>
  </li>
  <li class="nav-item active">
    <a href="#options" class="nav-link" role="tab" data-toggle="tab">Options</a>
  </li>
  <li class="nav-item">
    <a href="#team" class="nav-link" role="tab" data-toggle="tab">Team</a>
  </li>
</ul>
<div class="tab-content">
  <div id="home" role="tabpanel" class="tab-pane fade">
    Home
  </div>
  <div id="options" role="tabpanel" class="tab-pane fade show active">
    Options
  </div>
  <div id="team" role="tabpanel" class="tab-pane fade">
    Team
  </div>
</div>

@XhmikosR do not hesitate to add some details 👍