feat(Carousel): add indicatorLabels prop and fix indicator styling (#…

…5773)
react-bootstrap · May 20, 2021 · 0f7b39d · 0f7b39d
1 parent aba14d9
commit 0f7b39d
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 12 deletions.
diff --git a/src/Carousel.tsx b/src/Carousel.tsx
@@ -40,6 +40,7 @@ export interface CarouselProps
   fade?: boolean;
   controls?: boolean;
   indicators?: boolean;
+  indicatorLabels?: string[];
   activeIndex?: number;
   onSelect?: (eventKey: number, event: Record<string, unknown> | null) => void;
   defaultActiveIndex?: number;
@@ -85,6 +86,11 @@ const propTypes = {
    */
   indicators: PropTypes.bool,
 
+  /**
+   * An array of labels for the indicators. Defaults to "Slide #" if not provided.
+   */
+  indicatorLabels: PropTypes.array,
+
   /**
    * Controls the current visible slide
    *
@@ -174,7 +180,7 @@ const defaultProps = {
   fade: false,
   controls: true,
   indicators: true,
-
+  indicatorLabels: [],
   defaultActiveIndex: 0,
   interval: 5000,
   keyboard: true,
@@ -220,6 +226,7 @@ const Carousel: BsPrefixRefForwardingComponent<
     fade,
     controls,
     indicators,
+    indicatorLabels,
     activeIndex,
     onSelect,
     onSlide,
@@ -524,15 +531,23 @@ const Carousel: BsPrefixRefForwardingComponent<
       )}
     >
       {indicators && (
-        <ol className={`${prefix}-indicators`}>
-          {map(children, (_child, index) => (
-            <li
+        <div className={`${prefix}-indicators`}>
+          {map(children, (_, index) => (
+            <button
               key={index}
+              type="button"
+              data-bs-target="" // Bootstrap requires this in their css.
+              aria-label={
+                indicatorLabels?.length
+                  ? indicatorLabels[index]
+                  : `Slide ${index + 1}`
+              }
               className={index === renderedActiveIndex ? 'active' : undefined}
               onClick={indicatorOnClicks ? indicatorOnClicks[index] : undefined}
+              aria-current={index === renderedActiveIndex}
             />
           ))}
-        </ol>
+        </div>
       )}
 
       <div className={`${prefix}-inner`}>

diff --git a/test/CarouselSpec.js b/test/CarouselSpec.js
@@ -31,7 +31,7 @@ describe('<Carousel>', () => {
 
     expect(carouselItems.at(0).is('.active')).to.be.true;
     expect(carouselItems.at(1).is('.active')).to.be.false;
-    expect(wrapper.find('.carousel-indicators > li')).to.have.lengthOf(
+    expect(wrapper.find('.carousel-indicators > button')).to.have.lengthOf(
       items.length,
     );
   });
@@ -60,7 +60,7 @@ describe('<Carousel>', () => {
 
     expect(carouselItems.at(0).is('.active')).to.be.true;
     expect(carouselItems.at(0).text()).to.equal('Item 1 content');
-    expect(wrapper.find('.carousel-indicators > li')).to.have.lengthOf(2);
+    expect(wrapper.find('.carousel-indicators > button')).to.have.lengthOf(2);
   });
 
   it('should call onSelect when indicator selected', (done) => {
@@ -76,7 +76,23 @@ describe('<Carousel>', () => {
       </Carousel>,
     );
 
-    wrapper.find('.carousel-indicators li').first().simulate('click');
+    wrapper.find('.carousel-indicators button').first().simulate('click');
+  });
+
+  it('should render custom indicator labels', () => {
+    const labels = ['custom1', 'custom2', 'custom3'];
+
+    const wrapper = mount(
+      <Carousel activeIndex={1} interval={null} indicatorLabels={labels}>
+        {items}
+      </Carousel>,
+    );
+
+    const indicators = wrapper.find('.carousel-indicators button');
+    for (let i = 0; i < labels.length; i++) {
+      const node = indicators.at(i).getDOMNode();
+      expect(node.getAttribute('aria-label')).to.equal(labels[i]);
+    }
   });
 
   it('should render variant', () => {
@@ -137,7 +153,7 @@ describe('<Carousel>', () => {
         </Carousel>,
       );
 
-      wrapper.find('.carousel-indicators li').first().simulate('click');
+      wrapper.find('.carousel-indicators button').first().simulate('click');
     });
 
     it(`should call ${eventName} with next index and direction`, (done) => {
@@ -159,7 +175,7 @@ describe('<Carousel>', () => {
         </Carousel>,
       );
 
-      wrapper.find('.carousel-indicators li').last().simulate('click');
+      wrapper.find('.carousel-indicators button').last().simulate('click');
     });
   });
 
@@ -289,15 +305,15 @@ describe('<Carousel>', () => {
       <Carousel defaultActiveIndex={items.length - 1}>{items}</Carousel>,
     );
 
-    expect(wrapper.find('.carousel-indicators > li')).to.have.lengthOf(
+    expect(wrapper.find('.carousel-indicators > button')).to.have.lengthOf(
       items.length,
     );
 
     let fewerItems = items.slice(2);
 
     wrapper.setProps({ children: fewerItems });
 
-    expect(wrapper.find('.carousel-indicators > li')).to.have.lengthOf(
+    expect(wrapper.find('.carousel-indicators > button')).to.have.lengthOf(
       fewerItems.length,
     );
     expect(wrapper.find('div.carousel-item')).to.have.lengthOf(