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: use buttons, not links, for prev/next controls #32627

Merged
merged 20 commits into from Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f328d01
Carousel: use buttons, not links, for prev/next
patrickhlauke Dec 27, 2020
9116ff2
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Dec 28, 2020
b749662
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Dec 29, 2020
50ac915
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Dec 29, 2020
d753f4a
Use buttons for newly-merged extra carousel
patrickhlauke Dec 29, 2020
7b718ca
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Jan 5, 2021
13eb932
Edit carousel examples
patrickhlauke Jan 7, 2021
eda76bb
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Jan 7, 2021
c72d23b
Clarify that controls can be button or link
patrickhlauke Jan 7, 2021
1493ac0
Merge branch 'patrickhlauke-carousel' of https://github.com/twbs/boot…
patrickhlauke Jan 7, 2021
a2447ba
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Jan 10, 2021
7e1855d
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Jan 13, 2021
80f5812
Revert separate/unrelated change to CTA links
patrickhlauke Jan 13, 2021
c9cbb17
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Jan 15, 2021
6271d10
Merge branch 'main' into patrickhlauke-carousel
rohit2sharma95 Jan 19, 2021
c49d354
Update site/content/docs/5.0/components/carousel.md
patrickhlauke Jan 25, 2021
6373ac1
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Jan 25, 2021
e4414c7
Merge branch 'main' into patrickhlauke-carousel
XhmikosR Jan 26, 2021
47657bf
Merge branch 'main' into patrickhlauke-carousel
patrickhlauke Jan 27, 2021
3f0f00a
Explicitly set padding to 0 to prevent dipping/moving on active in Fi…
patrickhlauke Jan 27, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 31 additions & 7 deletions js/tests/unit/carousel.spec.js
Expand Up @@ -1154,16 +1154,40 @@ describe('Carousel', () => {
expect(Carousel.getInstance(carouselEl)).toBeDefined()
})

it('should create carousel and go to the next slide on click', done => {
it('should create carousel and go to the next slide on click (with real button controls)', done => {
fixtureEl.innerHTML = [
'<div id="myCarousel" class="carousel slide">',
' <div class="carousel-inner">',
' <div class="carousel-item active">item 1</div>',
' <div id="item2" class="carousel-item">item 2</div>',
' <div class="carousel-item">item 3</div>',
' </div>',
' <div class="carousel-control-prev" data-bs-target="#myCarousel" role="button" data-bs-slide="prev"></div>',
' <div id="next" class="carousel-control-next" data-bs-target="#myCarousel" role="button" data-bs-slide="next"></div>',
' <button class="carousel-control-prev" data-bs-target="#myCarousel" type="button" data-bs-slide="prev"></button>',
' <button id="next" class="carousel-control-next" data-bs-target="#myCarousel" type="button" data-bs-slide="next"></div>',
'</div>'
].join('')

const next = fixtureEl.querySelector('#next')
const item2 = fixtureEl.querySelector('#item2')

next.click()

setTimeout(() => {
expect(item2.classList.contains('active')).toEqual(true)
done()
}, 10)
})

it('should create carousel and go to the next slide on click (using links as controls)', done => {
fixtureEl.innerHTML = [
'<div id="myCarousel" class="carousel slide">',
' <div class="carousel-inner">',
' <div class="carousel-item active">item 1</div>',
' <div id="item2" class="carousel-item">item 2</div>',
' <div class="carousel-item">item 3</div>',
' </div>',
' <a class="carousel-control-prev" href="#myCarousel" role="button" data-bs-slide="prev"></button>',
patrickhlauke marked this conversation as resolved.
Show resolved Hide resolved
' <a id="next" class="carousel-control-next" href="#myCarousel" role="button" data-bs-slide="next"></div>',
'</div>'
].join('')

Expand Down Expand Up @@ -1209,8 +1233,8 @@ describe('Carousel', () => {
' <div class="carousel-item">item 2</div>',
' <div class="carousel-item">item 3</div>',
' </div>',
' <div class="carousel-control-prev" data-bs-target="#myCarousel" role="button" data-bs-slide="prev"></div>',
' <div id="next" class="carousel-control-next" role="button" data-bs-slide="next"></div>',
' <button class="carousel-control-prev" data-bs-target="#myCarousel" type="button" data-bs-slide="prev"></button>',
' <button id="next" class="carousel-control-next" type="button" data-bs-slide="next"></button>',
'</div>'
].join('')

Expand All @@ -1229,8 +1253,8 @@ describe('Carousel', () => {
' <div id="item2" class="carousel-item">item 2</div>',
' <div class="carousel-item">item 3</div>',
' </div>',
' <div class="carousel-control-prev" data-bs-target="#myCarousel" role="button" data-bs-slide="prev"></div>',
' <div id="next" class="carousel-control-next" data-bs-target="#myCarousel" role="button" data-bs-slide="next"></div>',
' <button class="carousel-control-prev" data-bs-target="#myCarousel" type="button" data-bs-slide="prev"></div>',
' <button id="next" class="carousel-control-next" data-bs-target="#myCarousel" type="button" data-bs-slide="next"></div>',
'</div>'
].join('')

Expand Down
8 changes: 4 additions & 4 deletions js/tests/visual/carousel.html
Expand Up @@ -34,14 +34,14 @@ <h1>Carousel <small>Bootstrap Visual Test</small></h1>
<img src="https://i.imgur.com/Nm7xoti.jpg" alt="Third slide">
</div>
</div>
<a class="carousel-control-prev" href="#carousel-example-generic" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" data-bs-target="#carousel-example-generic" type="button" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</a>
<a class="carousel-control-next" href="#carousel-example-generic" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" data-bs-target="#carousel-example-generic" type="button" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</a>
</button>
</div>
</div>

Expand Down
2 changes: 2 additions & 0 deletions scss/_carousel.scss
Expand Up @@ -100,6 +100,8 @@
width: $carousel-control-width;
color: $carousel-control-color;
text-align: center;
background: none;
border: 0;
opacity: $carousel-control-opacity;
@include transition($carousel-control-transition);

Expand Down
60 changes: 30 additions & 30 deletions site/content/docs/5.0/components/carousel.md
Expand Up @@ -22,7 +22,7 @@ Please be aware that nested carousels are not supported, and carousels are gener

Carousels don't automatically normalize slide dimensions. As such, you may need to use additional utilities or custom styles to appropriately size content. While carousels support previous/next controls and indicators, they're not explicitly required. Add and customize as you see fit.

**The `.active` class needs to be added to one of the slides** otherwise the carousel will not be visible. Also be sure to set a unique id on the `.carousel` for optional controls, especially if you're using multiple carousels on a single page. Control and indicator elements must have a `data-bs-target` attribute (or `href` for links) that matches the id of the `.carousel` element.
**The `.active` class needs to be added to one of the slides** otherwise the carousel will not be visible. Also be sure to set a unique `id` on the `.carousel` for optional controls, especially if you're using multiple carousels on a single page. Control and indicator elements must have a `data-bs-target` attribute (or `href` for links) that matches the `id` of the `.carousel` element.

### Slides only

Expand All @@ -46,7 +46,7 @@ Here's a carousel with slides only. Note the presence of the `.d-block` and `.w-

### With controls

Adding in the previous and next controls:
Adding in the previous and next controls. We recommend using `<button>` elements, but you can also use `<a>` link elements–though in that case, you should also add `role="button"` attributes to the links.
patrickhlauke marked this conversation as resolved.
Show resolved Hide resolved

{{< example >}}
<div id="carouselExampleControls" class="carousel slide" data-bs-ride="carousel">
Expand All @@ -61,14 +61,14 @@ Adding in the previous and next controls:
{{< placeholder width="800" height="400" class="bd-placeholder-img-lg d-block w-100" color="#333" background="#555" text="Third slide" >}}
</div>
</div>
<a class="carousel-control-prev" href="#carouselExampleControls" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" type="button" data-bs-target="#carouselExampleControls" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</a>
<a class="carousel-control-next" href="#carouselExampleControls" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" type="button" data-bs-target="#carouselExampleControls" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</a>
</button>
</div>
{{< /example >}}

Expand All @@ -94,14 +94,14 @@ You can also add the indicators to the carousel, alongside the controls, too.
{{< placeholder width="800" height="400" class="bd-placeholder-img-lg d-block w-100" color="#333" background="#555" text="Third slide" >}}
</div>
</div>
<a class="carousel-control-prev" href="#carouselExampleIndicators" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" type="button" data-bs-target="#carouselExampleIndicators" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</a>
<a class="carousel-control-next" href="#carouselExampleIndicators" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" type="button" data-bs-target="#carouselExampleIndicators" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</a>
</button>
</div>
{{< /example >}}

Expand Down Expand Up @@ -139,14 +139,14 @@ Add captions to your slides easily with the `.carousel-caption` element within a
</div>
</div>
</div>
<a class="carousel-control-prev" href="#carouselExampleCaptions" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" type="button" data-bs-target="#carouselExampleCaptions" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</a>
<a class="carousel-control-next" href="#carouselExampleCaptions" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" type="button" data-bs-target="#carouselExampleCaptions" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</a>
</button>
</div>
{{< /example >}}

Expand All @@ -167,14 +167,14 @@ Add `.carousel-fade` to your carousel to animate slides with a fade transition i
{{< placeholder width="800" height="400" class="bd-placeholder-img-lg d-block w-100" color="#333" background="#555" text="Third slide" >}}
</div>
</div>
<a class="carousel-control-prev" href="#carouselExampleFade" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" type="button" data-bs-target="#carouselExampleFade" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</a>
<a class="carousel-control-next" href="#carouselExampleFade" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" type="button" data-bs-target="#carouselExampleFade" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</a>
</button>
</div>
{{< /example >}}

Expand All @@ -195,14 +195,14 @@ Add `data-bs-interval=""` to a `.carousel-item` to change the amount of time to
{{< placeholder width="800" height="400" class="bd-placeholder-img-lg d-block w-100" color="#333" background="#555" text="Third slide" >}}
</div>
</div>
<a class="carousel-control-prev" href="#carouselExampleInterval" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" type="button" data-bs-target="#carouselExampleInterval" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</a>
<a class="carousel-control-next" href="#carouselExampleInterval" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" type="button" data-bs-target="#carouselExampleInterval" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</a>
</button>
</div>
{{< /example >}}

Expand All @@ -223,14 +223,14 @@ Carousels support swiping left/right on touchscreen devices to move between slid
{{< placeholder width="800" height="400" class="bd-placeholder-img-lg d-block w-100" color="#333" background="#555" text="Third slide" >}}
</div>
</div>
<a class="carousel-control-prev" href="#carouselExampleControlsNoTouching" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" type="button" data-bs-target="#carouselExampleControlsNoTouching" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</a>
<a class="carousel-control-next" href="#carouselExampleControlsNoTouching" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" type="button" data-bs-target="#carouselExampleControlsNoTouching" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</a>
</button>
</div>
{{< /example >}}

Expand Down Expand Up @@ -268,14 +268,14 @@ Add `.carousel-dark` to the `.carousel` for darker controls, indicators, and cap
</div>
</div>
</div>
<a class="carousel-control-prev" href="#carouselExampleDark" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" type="button" data-bs-target="#carouselExampleDark" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</a>
<a class="carousel-control-next" href="#carouselExampleDark" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" type="button" data-bs-target="#carouselExampleDark" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</a>
</button>
</div>
{{< /example >}}

Expand Down
8 changes: 4 additions & 4 deletions site/content/docs/5.0/examples/carousel-rtl/index.html
Expand Up @@ -74,14 +74,14 @@ <h1>واحد أكثر لقياس جيد.</h1>
</div>
</div>
</div>
<a class="carousel-control-prev" href="#myCarousel" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" type="button" data-bs-target="#myCarousel" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">السابق</span>
</a>
<a class="carousel-control-next" href="#myCarousel" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" type="button" data-bs-target="#myCarousel" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">التالى</span>
</a>
</button>
</div>


Expand Down
8 changes: 4 additions & 4 deletions site/content/docs/5.0/examples/carousel/index.html
Expand Up @@ -73,14 +73,14 @@ <h1>One more for good measure.</h1>
</div>
</div>
</div>
<a class="carousel-control-prev" href="#myCarousel" role="button" data-bs-slide="prev">
<button class="carousel-control-prev" type="button" data-bs-target="#myCarousel" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</a>
<a class="carousel-control-next" href="#myCarousel" role="button" data-bs-slide="next">
</button>
<button class="carousel-control-next" type="button" data-bs-target="#myCarousel" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</a>
</button>
</div>


Expand Down