From 9ad371da6f394e1cd5d8624a735fd24bc567d34d Mon Sep 17 00:00:00 2001
From: langemike <mikevv@gmail.com>
Date: Mon, 25 Nov 2024 12:27:36 +0100
Subject: [PATCH] fix: accessibility improvements for Hero shelf pagination and
 LayoutGrid

* fix(a11y): rowindex and colindex must start from 1

* fix(a11y): hero shelf pagination list aria markup
---
 .../__snapshots__/CardGrid.test.tsx.snap      |  8 +-
 .../__snapshots__/Favorites.test.tsx.snap     |  8 +-
 .../HeroShelf/HeroShelf.module.scss           |  4 +-
 .../HeroShelf/HeroShelfPagination.tsx         | 82 ++++++++++---------
 .../src/components/LayoutGrid/LayoutGrid.tsx  |  4 +-
 .../User/__snapshots__/User.test.tsx.snap     |  8 +-
 6 files changed, 58 insertions(+), 56 deletions(-)

diff --git a/packages/ui-react/src/components/CardGrid/__snapshots__/CardGrid.test.tsx.snap b/packages/ui-react/src/components/CardGrid/__snapshots__/CardGrid.test.tsx.snap
index 7eb8d1a4d..888929dbd 100644
--- a/packages/ui-react/src/components/CardGrid/__snapshots__/CardGrid.test.tsx.snap
+++ b/packages/ui-react/src/components/CardGrid/__snapshots__/CardGrid.test.tsx.snap
@@ -9,12 +9,12 @@ exports[`<CardGrid> > renders and matches snapshot 1`] = `
       role="grid"
     >
       <div
-        aria-rowindex="0"
+        aria-rowindex="1"
         class="_row_19b484"
         role="row"
       >
         <div
-          aria-colindex="0"
+          aria-colindex="1"
           class="_cell_19b484"
           id="layout_grid_0-0"
           role="gridcell"
@@ -56,7 +56,7 @@ exports[`<CardGrid> > renders and matches snapshot 1`] = `
           </a>
         </div>
         <div
-          aria-colindex="1"
+          aria-colindex="2"
           class="_cell_19b484"
           id="layout_grid_0-1"
           role="gridcell"
@@ -98,7 +98,7 @@ exports[`<CardGrid> > renders and matches snapshot 1`] = `
           </a>
         </div>
         <div
-          aria-colindex="2"
+          aria-colindex="3"
           class="_cell_19b484"
           id="layout_grid_0-2"
           role="gridcell"
diff --git a/packages/ui-react/src/components/Favorites/__snapshots__/Favorites.test.tsx.snap b/packages/ui-react/src/components/Favorites/__snapshots__/Favorites.test.tsx.snap
index 3b3108490..63e8e5cab 100644
--- a/packages/ui-react/src/components/Favorites/__snapshots__/Favorites.test.tsx.snap
+++ b/packages/ui-react/src/components/Favorites/__snapshots__/Favorites.test.tsx.snap
@@ -25,12 +25,12 @@ exports[`<Favorites> > renders and matches snapshot 1`] = `
         role="grid"
       >
         <div
-          aria-rowindex="0"
+          aria-rowindex="1"
           class="_row_19b484"
           role="row"
         >
           <div
-            aria-colindex="0"
+            aria-colindex="1"
             class="_cell_19b484"
             id="layout_grid_0-0"
             role="gridcell"
@@ -77,7 +77,7 @@ exports[`<Favorites> > renders and matches snapshot 1`] = `
             </a>
           </div>
           <div
-            aria-colindex="1"
+            aria-colindex="2"
             class="_cell_19b484"
             id="layout_grid_0-1"
             role="gridcell"
@@ -124,7 +124,7 @@ exports[`<Favorites> > renders and matches snapshot 1`] = `
             </a>
           </div>
           <div
-            aria-colindex="2"
+            aria-colindex="3"
             class="_cell_19b484"
             id="layout_grid_0-2"
             role="gridcell"
diff --git a/packages/ui-react/src/components/HeroShelf/HeroShelf.module.scss b/packages/ui-react/src/components/HeroShelf/HeroShelf.module.scss
index be328ccd5..dab11c7e7 100644
--- a/packages/ui-react/src/components/HeroShelf/HeroShelf.module.scss
+++ b/packages/ui-react/src/components/HeroShelf/HeroShelf.module.scss
@@ -285,7 +285,7 @@ $mobile-landscape-height: 100vh;
   }
 
   &:hover {
-    transform: scale(1.2) translateY(-50%);;
+    transform: scale(1.2) translateY(-50%);
   }
 
   &:disabled {
@@ -361,7 +361,7 @@ $mobile-landscape-height: 100vh;
     }
   }
 
-  &[aria-hidden="true"] {
+  &[disabled] {
     cursor: default;
   }
 }
diff --git a/packages/ui-react/src/components/HeroShelf/HeroShelfPagination.tsx b/packages/ui-react/src/components/HeroShelf/HeroShelfPagination.tsx
index cd2e04575..c27a30e59 100644
--- a/packages/ui-react/src/components/HeroShelf/HeroShelfPagination.tsx
+++ b/packages/ui-react/src/components/HeroShelf/HeroShelfPagination.tsx
@@ -50,53 +50,55 @@ const HeroShelfPagination = ({ playlist, index: indexIn, direction, nextIndex: n
   }, [playlist.playlist, placeholderCount]);
 
   return (
-    <nav className={styles.dots}>
+    <>
       <div aria-live="polite" className="hidden">
         {t('slide_indicator', { page: indexIn + 1, pages: playlist.playlist.length })}
       </div>
-      <ul className={styles.dotsList}>
-        {playlistWithPlaceholders.map((current, itemIndex) => {
-          if (itemIndex < index - range - 1 || itemIndex > index + range + 1) {
-            return null;
-          }
-          if (!current) {
-            return <div className={classNames(styles.dotPlaceholder)} key={itemIndex} aria-hidden="true" />;
-          }
+      <nav className={styles.dots}>
+        <ul className={styles.dotsList}>
+          {playlistWithPlaceholders.map((current, itemIndex) => {
+            if (itemIndex < index - range - 1 || itemIndex > index + range + 1) {
+              return null;
+            }
+            if (!current) {
+              return <li className={classNames(styles.dotPlaceholder)} key={itemIndex} aria-hidden="true" />;
+            }
 
-          const movementBase = 22; // dot width (10) + gap(12)
-          const movementTotal = Math.abs(index - nextIndex) * movementBase;
-          const movement = direction === 'left' ? movementTotal : direction === 'right' ? 0 - movementTotal : 0;
-          const transform = `translateX(${movement}px)`;
-          const transition = direction
-            ? `transform ${animationDuration}ms ease-out ${animationDuration / 3}ms, width ${animationDuration / 2}ms ease-out ${animationDuration / 3}ms`
-            : '';
+            const movementBase = 22; // dot width (10) + gap(12)
+            const movementTotal = Math.abs(index - nextIndex) * movementBase;
+            const movement = direction === 'left' ? movementTotal : direction === 'right' ? 0 - movementTotal : 0;
+            const transform = `translateX(${movement}px)`;
+            const transition = direction
+              ? `transform ${animationDuration}ms ease-out ${animationDuration / 3}ms, width ${animationDuration / 2}ms ease-out ${animationDuration / 3}ms`
+              : '';
 
-          const size = calculateDotSize(direction, itemIndex, index, range, 0.6);
-          const transformDiv = `scale(${size})`;
-          const transitionDiv = direction
-            ? `width ${animationDuration}ms ease-out, height ${animationDuration}ms ease-out, transform ${animationDuration}ms ease-out`
-            : '';
+            const size = calculateDotSize(direction, itemIndex, index, range, 0.6);
+            const transformDiv = `scale(${size})`;
+            const transitionDiv = direction
+              ? `width ${animationDuration}ms ease-out, height ${animationDuration}ms ease-out, transform ${animationDuration}ms ease-out`
+              : '';
 
-          const isCurrent = itemIndex === index;
-          const hidden = size !== 1;
-          const ariaLabel = hidden ? undefined : t('slide_to', { item: itemIndex - placeholderCount + 1, items: playlist.playlist.length });
+            const isCurrent = itemIndex === index;
+            const hidden = size !== 1;
+            const ariaLabel = hidden ? undefined : t('slide_to', { item: itemIndex - placeholderCount + 1, items: playlist.playlist.length });
 
-          return (
-            <button
-              key={current?.mediaid}
-              className={classNames(styles.dot, itemIndex === nextIndex && styles.dotActive, !direction && itemIndex === index && styles.dotActive)}
-              style={{ transform, transition }}
-              aria-label={ariaLabel}
-              aria-hidden={hidden ? 'true' : undefined}
-              aria-current={isCurrent ? 'true' : undefined}
-              onClick={hidden || isCurrent ? undefined : () => setIndex(itemIndex - placeholderCount)}
-            >
-              <div style={{ transform: transformDiv, transition: transitionDiv }} />
-            </button>
-          );
-        })}
-      </ul>
-    </nav>
+            return (
+              <li key={current?.mediaid} aria-hidden={hidden || undefined} aria-current={isCurrent || undefined}>
+                <button
+                  className={classNames(styles.dot, itemIndex === nextIndex && styles.dotActive, !direction && itemIndex === index && styles.dotActive)}
+                  style={{ transform, transition }}
+                  aria-label={ariaLabel}
+                  disabled={hidden || undefined}
+                  onClick={hidden || isCurrent ? undefined : () => setIndex(itemIndex - placeholderCount)}
+                >
+                  <div style={{ transform: transformDiv, transition: transitionDiv }} />
+                </button>
+              </li>
+            );
+          })}
+        </ul>
+      </nav>
+    </>
   );
 };
 
diff --git a/packages/ui-react/src/components/LayoutGrid/LayoutGrid.tsx b/packages/ui-react/src/components/LayoutGrid/LayoutGrid.tsx
index f3e337db6..e77431452 100644
--- a/packages/ui-react/src/components/LayoutGrid/LayoutGrid.tsx
+++ b/packages/ui-react/src/components/LayoutGrid/LayoutGrid.tsx
@@ -116,13 +116,13 @@ const LayoutGrid = <Item extends object>({ className, columnCount, data, renderC
   return (
     <div role="grid" ref={gridRef} aria-rowcount={rowCount} className={className} onKeyDown={handleKeyDown}>
       {Array.from({ length: rowCount }).map((_, rowIndex) => (
-        <div role="row" key={rowIndex} aria-rowindex={rowIndex} className={styles.row}>
+        <div role="row" key={rowIndex} aria-rowindex={rowIndex + 1} className={styles.row}>
           {data.slice(rowIndex * columnCount, rowIndex * columnCount + columnCount).map((item, columnIndex) => (
             <div
               role="gridcell"
               id={`layout_grid_${rowIndex}-${columnIndex}`}
               key={getCellKey(item)}
-              aria-colindex={columnIndex}
+              aria-colindex={columnIndex + 1}
               className={styles.cell}
               style={gridCellStyle}
             >
diff --git a/packages/ui-react/src/pages/User/__snapshots__/User.test.tsx.snap b/packages/ui-react/src/pages/User/__snapshots__/User.test.tsx.snap
index aa8fe5729..a7d60dbac 100644
--- a/packages/ui-react/src/pages/User/__snapshots__/User.test.tsx.snap
+++ b/packages/ui-react/src/pages/User/__snapshots__/User.test.tsx.snap
@@ -416,12 +416,12 @@ exports[`User Component tests > Favorites Page 1`] = `
             role="grid"
           >
             <div
-              aria-rowindex="0"
+              aria-rowindex="1"
               class="_row_19b484"
               role="row"
             >
               <div
-                aria-colindex="0"
+                aria-colindex="1"
                 class="_cell_19b484"
                 id="layout_grid_0-0"
                 role="gridcell"
@@ -463,7 +463,7 @@ exports[`User Component tests > Favorites Page 1`] = `
                 </a>
               </div>
               <div
-                aria-colindex="1"
+                aria-colindex="2"
                 class="_cell_19b484"
                 id="layout_grid_0-1"
                 role="gridcell"
@@ -505,7 +505,7 @@ exports[`User Component tests > Favorites Page 1`] = `
                 </a>
               </div>
               <div
-                aria-colindex="2"
+                aria-colindex="3"
                 class="_cell_19b484"
                 id="layout_grid_0-2"
                 role="gridcell"