fix: small a11y semantic improvements (#802)

* fix: small a11y semantic improvements * fix: update unit tests * fix: revert change to demo announce slide * fix: add aria-selected to default-controls
FormidableLabs · Sep 21, 2021 · 4964ab1 · 4964ab1
1 parent 4cdc5d6
commit 4964ab1
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 29 deletions.
diff --git a/src/announce-slide.js b/src/announce-slide.js
@@ -9,7 +9,7 @@ const AnnounceSlide = ({ message }) => {
     padding: 0,
     margin: '-1px',
     clip: 'rect(0, 0, 0, 0)',
-    'white-space': 'nowrap',
+    whiteSpace: 'nowrap',
     border: 0
   };
   return (

diff --git a/src/default-controls.js b/src/default-controls.js
@@ -240,6 +240,7 @@ export const PagingDots = (props) => {
               }}
               onClick={props.goToSlide.bind(null, index)}
               aria-label={`slide ${index + 1} bullet`}
+              aria-selected={isActive}
             >
               <svg
                 className="paging-dot"

diff --git a/src/index.js b/src/index.js
@@ -1127,7 +1127,7 @@ export default class Carousel extends React.Component {
   }
 
   render() {
-    const { currentSlide, slideCount, frameWidth } = this.state;
+    const { currentSlide, slideCount, frameWidth, hasInteraction } = this.state;
     const {
       disableAnimation,
       frameOverflow,
@@ -1142,7 +1142,7 @@ export default class Carousel extends React.Component {
         this.state.resetWrapAroundPosition &&
         this.props.wrapAround) ||
       disableAnimation ||
-      !this.state.hasInteraction
+      !hasInteraction
         ? 0
         : this.props.speed;
 
@@ -1162,9 +1162,11 @@ export default class Carousel extends React.Component {
     } = this.getOffsetDeltas();
 
     return (
-      <div
+      <section
         className={['slider', this.props.className || ''].join(' ').trim()}
         onFocus={this.handleFocus}
+        aria-label="carousel-slider"
+        role="region"
         onBlur={this.handleBlur}
         ref={this.props.innerRef}
         tabIndex={0}
@@ -1260,7 +1262,7 @@ export default class Carousel extends React.Component {
             dangerouslySetInnerHTML={{ __html: getImgTagStyles() }}
           />
         )}
-      </div>
+      </section>
     );
   }
 }

diff --git a/src/transitions/3d-scroll-transition.js b/src/transitions/3d-scroll-transition.js
@@ -55,17 +55,19 @@ export default class ScrollTransition3D extends React.Component {
       const visible = this.getDistanceToCurrentSlide(index) <= slidesToShow / 2;
       const current = currentSlide === index;
       return (
-        <li
+        <div
           className={`slider-slide${visible ? ' slide-visible' : ''}${
             current ? ' slide-current' : ''
           }`}
           style={this.getSlideStyles(index, positionValue)}
           key={index}
+          aria-label={`slide ${index + 1} of ${children.length}`}
+          role="group"
           onClick={handleSelfFocus}
           tabIndex={-1}
         >
           {child}
-        </li>
+        </div>
       );
     });
   }
@@ -216,9 +218,9 @@ export default class ScrollTransition3D extends React.Component {
   render() {
     const children = this.formatChildren(this.props.children);
     return (
-      <ul className="slider-list" style={this.getListStyles()}>
+      <div className="slider-list" style={this.getListStyles()}>
         {children}
-      </ul>
+      </div>
     );
   }
 }

diff --git a/src/transitions/fade-transition.js b/src/transitions/fade-transition.js
@@ -12,19 +12,21 @@ export default class FadeTransition extends React.Component {
   formatChildren(children, opacity) {
     const { currentSlide, slidesToShow } = this.props;
     return React.Children.map(children, (child, index) => (
-      <li
+      <div
         className={`slider-slide${getSlideClassName(
           index,
           currentSlide,
           slidesToShow
         )}`}
         style={this.getSlideStyles(index, opacity)}
         key={index}
+        aria-label={`slide ${index + 1} of ${children.length}`}
+        role="group"
         onClick={handleSelfFocus}
         tabIndex={-1}
       >
         {child}
-      </li>
+      </div>
     ));
   }
 
@@ -126,9 +128,9 @@ export default class FadeTransition extends React.Component {
     );
 
     return (
-      <ul className="slider-list" style={this.getContainerStyles()}>
+      <div className="slider-list" style={this.getContainerStyles()}>
         {children}
-      </ul>
+      </div>
     );
   }
 }

diff --git a/src/transitions/scroll-transition.js b/src/transitions/scroll-transition.js
@@ -128,20 +128,22 @@ export default class ScrollTransition extends React.Component {
       const isVisible = isFullyVisible(index, this.props);
       const inert = isVisible ? {} : { inert: 'true' };
       return (
-        <li
+        <div
           className={`slider-slide${getSlideClassName(
             index,
             currentSlide,
             slidesToShow
           )}`}
+          aria-label={`slide ${index + 1} of ${children.length}`}
+          role="group"
           style={this.getSlideStyles(index, positionValue)}
           key={index}
           onClick={handleSelfFocus}
           tabIndex={-1}
           {...inert}
         >
           {child}
-        </li>
+        </div>
       );
     });
   }
@@ -217,12 +219,12 @@ export default class ScrollTransition extends React.Component {
     const deltaY = this.props.deltaY;
 
     return (
-      <ul
+      <div
         className="slider-list"
         style={this.getListStyles({ deltaX, deltaY })}
       >
         {children}
-      </ul>
+      </div>
     );
   }
 }

diff --git a/test/specs/carousel.test.js b/test/specs/carousel.test.js
@@ -16,15 +16,15 @@ describe('<Carousel />', () => {
       expect(children).toHaveLength(3);
     });
 
-    it('should render a div with the class `slider`.', () => {
+    it('should render a section with the class `slider`.', () => {
       const wrapper = mount(
         <Carousel>
           <p>Slide 1</p>
           <p>Slide 2</p>
           <p>Slide 3</p>
         </Carousel>
       );
-      const sliderDiv = wrapper.find('div.slider');
+      const sliderDiv = wrapper.find('section.slider');
       expect(sliderDiv).toHaveLength(1);
     });
 
@@ -40,15 +40,15 @@ describe('<Carousel />', () => {
       expect(sliderFrameDiv).toHaveLength(1);
     });
 
-    it('should render an ul with the class `slider-list`.', () => {
+    it('should render a div with the class `slider-list`.', () => {
       const wrapper = mount(
         <Carousel>
           <p>Slide 1</p>
           <p>Slide 2</p>
           <p>Slide 3</p>
         </Carousel>
       );
-      const sliderListUl = wrapper.find('ul.slider-list');
+      const sliderListUl = wrapper.find('div.slider-list');
       expect(sliderListUl).toHaveLength(1);
     });
 
@@ -137,7 +137,7 @@ describe('<Carousel />', () => {
           <p>Slide 3</p>
         </Carousel>
       );
-      expect(wrapper.find('div.slider').instance()).toEqual(ref.current);
+      expect(wrapper.find('section.slider').instance()).toEqual(ref.current);
     });
   });
 
@@ -171,7 +171,7 @@ describe('<Carousel />', () => {
           <p>Slide 3</p>
         </Carousel>
       );
-      const slider = wrapper.find('div.slider');
+      const slider = wrapper.find('section.slider');
       expect(slider).toHaveLength(1);
     });
 
@@ -183,7 +183,7 @@ describe('<Carousel />', () => {
           <p>Slide 3</p>
         </Carousel>
       );
-      const sliderList = wrapper.find('ul.slider-list');
+      const sliderList = wrapper.find('div.slider-list');
       expect(sliderList.prop('style').transform).toBe(
         'translate3d(-100px, 0px, 0)'
       );
@@ -239,7 +239,7 @@ describe('<Carousel />', () => {
           <p>Slide 3</p>
         </Carousel>
       );
-      const slider = wrapper.find('div.test');
+      const slider = wrapper.find('section.test');
       expect(slider).toHaveLength(1);
     });
 
@@ -263,7 +263,7 @@ describe('<Carousel />', () => {
           <p>Slide 3</p>
         </Carousel>
       );
-      const slider = wrapper.find('div.slider');
+      const slider = wrapper.find('section.slider');
       expect(slider).toHaveStyle('backgroundColor', 'black');
       expect(slider).toHaveStyle('display', 'block');
     });
@@ -619,7 +619,7 @@ describe('<Carousel />', () => {
         );
 
         // Setting carousel focus
-        wrapper.find('div.slider').simulate('focus');
+        wrapper.find('section.slider').simulate('focus');
 
         expect(wrapper).toHaveState({ currentSlide: 0 });
         map.keydown({ keyCode: 39 });
@@ -641,7 +641,7 @@ describe('<Carousel />', () => {
         );
 
         // Setting carousel focus
-        wrapper.find('div.slider').simulate('focus');
+        wrapper.find('section.slider').simulate('focus');
 
         expect(wrapper).toHaveState({ currentSlide: 0 });
         map.keydown({ keyCode: 39 });
@@ -669,7 +669,7 @@ describe('<Carousel />', () => {
           );
 
           // Setting carousel focus
-          wrapper.find('div.slider').simulate('focus');
+          wrapper.find('section.slider').simulate('focus');
 
           expect(wrapper).toHaveState({ currentSlide: 0 });
           map.keydown({ keyCode: 38 });