Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

v4 list-group not work correctly with tabs #20620

Closed
agavazov opened this issue Sep 3, 2016 · 10 comments
Closed

v4 list-group not work correctly with tabs #20620

agavazov opened this issue Sep 3, 2016 · 10 comments
Labels
js v4
Milestone
v4.0.0-beta

Comments

@agavazov
Copy link

agavazov commented Sep 3, 2016 •
edited
Loading

On V3 when I make tabs with list-group it works fine, but in V4 tabs are open once and active class does not disappear.

This example is with V3, you can see whats happen when test it with V4

<html>
<head>
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
    <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js"></script>
    <link href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css" rel="stylesheet"/>
</head>
<body>

<ul class="list-group">
    <li class="list-group-item active"><a href="#panel1" data-toggle="tab">Open panel 1</a></li>
    <li class="list-group-item"><a href="#panel2" data-toggle="tab">Open panel 2</a></li>
</ul>

<hr/>

<div class="tab-content">
    <div class="tab-pane active" id="panel1">
        <div class="panel-body">Panel 1 body</div>
    </div>
    <div class="tab-pane" id="panel2">
        <div class="panel-body">Panel 2 body</div>
    </div>
</div>

</body>
</html>
@agavazov
Copy link
Author

agavazov commented Sep 3, 2016

You can see the same problem with this example http://www.codeply.com/go/rv3Cvr0lZ4

@mdo
Copy link
Member

mdo commented Oct 27, 2016

Yeah, right now our tab JS only works with navigation components I think, not the list group. Unsure how complicated it'd be to add support for that, but maybe our newest JS friends @Johann-S and @bardiharborow could give us an idea of if it's doable?

@Johann-S
Copy link
Member

Johann-S commented Oct 28, 2016 •
edited
Loading

It's because of this line : https://github.com/twbs/bootstrap/blob/v4-dev/js/src/tab.js#L50

IMO If we change this selector to accept .list-group-item it will work fine

You can make it works like this, add .nav-item to li :

  <ul class="list-group">
    <li class="list-group-item nav-item">
      <a class="nav-link active" data-toggle="tab" href="#homeJo" role="tab">Home</a>
    </li>
    <li class="list-group-item nav-item">
      <a class="nav-link" data-toggle="tab" href="#profileJo" role="tab">Profile</a>
    </li>
  </ul>

  <!-- Tab panes -->
  <div class="tab-content">
    <div class="tab-pane active" id="homeJo" role="tabpanel">Home</div>
    <div class="tab-pane" id="profileJo" role="tabpanel">Profile</div>
  </div>

Is it ok for you @mdo my solution ?

@mdo
Copy link
Member

mdo commented Oct 28, 2016

You can make it works like this, add .nav-item to li

Hmmm, maybe. We could also update that line of JS to accept multiple selectors, right? I could go either way tbh. If we do the multiple selectors for our JS elsewhere, then I'd say let's do that. But if we encourage mixing/matching of classes across different components, than let's do that.

Thoughts?

@Johann-S
Copy link
Member

It's a tough choice, FYI I didn't find any multiple selectors in our JS currently and tbh it's difficult to know which solution is the better one 😞

@mdo mdo mentioned this issue Nov 28, 2016
Closed
7 tasks
@mdo mdo added this to the v4.0.0-alpha.6 milestone Nov 28, 2016
@giulianovelli
Copy link

Maybe having specific selectors for behavior as opposed to styles could be a more generic and elegant solution? This way they could be applied to any element and it wouldn't seem weird. Someone differentiate these selectors adding a prefix like js- to the name.

@mdo mdo modified the milestones: v4.0.0-beta, v4.0.0-alpha.6 Dec 27, 2016
@Johann-S
Copy link
Member

Finally I think we should be more flexible with our components because personnaly I prefer a flexible framework than a rigid framework.
So we should allow Tab JS to work with all lists provide by Bootstrap

@mdo
Copy link
Member

mdo commented Dec 31, 2016

I agree with that @Johann-S! Any ideas?

@Johann-S
Copy link
Member

It's wrong what I said previously we already have multi selectors in our JS in Tab JS. So we have to expand this selector to accept all lists provided by Bootstrap.

@Johann-S Johann-S mentioned this issue Jan 17, 2017
Merged
@pvdlg pvdlg mentioned this issue Jan 22, 2017
Merged
@mdo
Copy link
Member

mdo commented Mar 18, 2017

Fixed by #21756.

@mdo mdo closed this as completed Mar 18, 2017
@mdo mdo mentioned this issue Mar 18, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
js v4
Projects
None yet
Milestone
v4.0.0-beta
Development

No branches or pull requests
4 participants
@mdo @agavazov @giulianovelli @Johann-S