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.

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

Carousel Indicators not showing in v12.0.0-beta4 #4200

Closed
Swanzor opened this issue Dec 14, 2021 · 0 comments · Fixed by #4259
Closed

Carousel Indicators not showing in v12.0.0-beta4 #4200

Swanzor opened this issue Dec 14, 2021 · 0 comments · Fixed by #4259

Comments

@Swanzor
Copy link

Swanzor commented Dec 14, 2021

Bug description:

There seems to be an issue with with carousel indicators not being styled and therefore they do not show at all. After some investigation on my own it seems there is a missing attribute of data-bs-target on the indicator (I personally believe this is a mistake in Bootstrap, but it is what it is) as you see from the snippet here (you can locate this on line 158 - 196 in the _carousel.scss file in the Bootstrap source):

.carousel-indicators {
  ...

  [data-bs-target] {
    ...
  }

  ...
}

You can see this in action https://getbootstrap.com/docs/5.1/components/carousel/#with-indicators on the Bootstrap 5 docs.

To replicate this simply create any carousel with 2 or more slides and you will see in ng-bootstrap 12.0.0-beta4 spit out the following result:

<ol role="tablist" class="carousel-indicators">
  <li role="tab" class="active" aria-labelledby="slide-ngb-slide-0" aria-controls="slide-ngb-slide-0" aria-selected="true"></li>
  <li role="tab" class="" aria-labelledby="slide-ngb-slide-1" aria-controls="slide-ngb-slide-1" aria-selected="false" ></li>
  ...
</ol>

As you can see the attribute data-bs-target is absent. This means that no indicators will show even tough they are styled. Seems like a quick fix to just add the attribute to the template.

Versions of Angular, ng-bootstrap and Bootstrap:

Angular: v13.1.0

ng-bootstrap: v12.0.0-beta.4

Bootstrap: v5.1.3

@maxokorokov maxokorokov added this to the 12.0.1 milestone Feb 21, 2022
@maxokorokov maxokorokov linked a pull request Feb 21, 2022 that will close this issue
maxokorokov added a commit to maxokorokov/ng-bootstrap that referenced this issue Feb 22, 2022
- In Bootstrap 5 the CSS selector matching indicators has changed and included `data-bs-target` attribute that we were missing
- Recommended carousel markup changed from using `ul`, `li` and `a` to using `button` elements
- Had to refactor tests as selectors were not precise enough

On Bootstrap side see:
- twbs/bootstrap#32661
- twbs/bootstrap#32627

Fixes ng-bootstrap#4200
Fixes ng-bootstrap#4253
maxokorokov added a commit that referenced this issue Feb 23, 2022
- In Bootstrap 5 the CSS selector matching indicators has changed and included `data-bs-target` attribute that we were missing
- Recommended carousel markup changed from using `ul`, `li` and `a` to using `button` elements
- Had to refactor tests as selectors were not precise enough

On Bootstrap side see:
- twbs/bootstrap#32661
- twbs/bootstrap#32627

Fixes #4200
Fixes #4253
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants