fix(Nav): allow RTL scrolling (#9637)

* fix(Nav): allow rtl scrolling * add direction assignment to update * update aria label descriptions * add new props and deprecate old * update desc * update other test snaps
patternfly · Sep 21, 2023 · faddb9c · faddb9c
1 parent 262e474
commit faddb9c
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 28 deletions.
diff --git a/packages/react-core/src/components/Nav/NavList.tsx b/packages/react-core/src/components/Nav/NavList.tsx
@@ -14,10 +14,14 @@ export interface NavListProps
   children?: React.ReactNode;
   /** Additional classes added to the list */
   className?: string;
-  /** Aria-label for the left scroll button */
+  /** @deprecated Please use backScrollAriaLabel. Aria-label for the left scroll button */
   ariaLeftScroll?: string;
-  /** Aria-label for the right scroll button */
+  /** @deprecated Please use forwardScrollAriaLabel. Aria-label for the right scroll button */
   ariaRightScroll?: string;
+  /** Aria-label for the back scroll button */
+  backScrollAriaLabel?: string;
+  /** Aria-label for the forward scroll button */
+  forwardScrollAriaLabel?: string;
 }
 
 class NavList extends React.Component<NavListProps> {
@@ -26,8 +30,11 @@ class NavList extends React.Component<NavListProps> {
   context!: React.ContextType<typeof NavContext>;
   static defaultProps: NavListProps = {
     ariaLeftScroll: 'Scroll left',
-    ariaRightScroll: 'Scroll right'
+    backScrollAriaLabel: 'Scroll back',
+    ariaRightScroll: 'Scroll right',
+    forwardScrollAriaLabel: 'Scroll foward'
   };
+  private direction = 'ltr';
 
   state = {
     scrollViewAtStart: false,
@@ -51,7 +58,7 @@ class NavList extends React.Component<NavListProps> {
     }
   };
 
-  scrollLeft = () => {
+  scrollBack = () => {
     // find first Element that is fully in view on the left, then scroll to the element before it
     const container = this.navList.current;
     if (container) {
@@ -65,13 +72,19 @@ class NavList extends React.Component<NavListProps> {
         }
       }
       if (lastElementOutOfView) {
-        container.scrollLeft -= lastElementOutOfView.scrollWidth;
+        if (this.direction === 'ltr') {
+          // LTR scrolls left to go back
+          container.scrollLeft -= lastElementOutOfView.scrollWidth;
+        } else {
+          // RTL scrolls right to go back
+          container.scrollLeft += lastElementOutOfView.scrollWidth;
+        }
       }
       this.handleScrollButtons();
     }
   };
 
-  scrollRight = () => {
+  scrollForward = () => {
     // find last Element that is fully in view on the right, then scroll to the element after it
     const container = this.navList.current;
     if (container) {
@@ -85,23 +98,42 @@ class NavList extends React.Component<NavListProps> {
         }
       }
       if (firstElementOutOfView) {
-        container.scrollLeft += firstElementOutOfView.scrollWidth;
+        if (this.direction === 'ltr') {
+          // LTR scrolls right to go forward
+          container.scrollLeft += firstElementOutOfView.scrollWidth;
+        } else {
+          // RTL scrolls left to go forward
+          container.scrollLeft -= firstElementOutOfView.scrollWidth;
+        }
       }
       this.handleScrollButtons();
     }
   };
 
   componentDidMount() {
     this.observer = getResizeObserver(this.navList.current, this.handleScrollButtons, true);
+    this.direction = getComputedStyle(this.navList.current).getPropertyValue('direction');
     this.handleScrollButtons();
   }
 
   componentWillUnmount() {
     this.observer();
   }
 
+  componentDidUpdate() {
+    this.direction = getComputedStyle(this.navList.current).getPropertyValue('direction');
+  }
+
   render() {
-    const { children, className, ariaLeftScroll, ariaRightScroll, ...props } = this.props;
+    const {
+      children,
+      className,
+      ariaLeftScroll,
+      ariaRightScroll,
+      backScrollAriaLabel,
+      forwardScrollAriaLabel,
+      ...props
+    } = this.props;
     const { scrollViewAtStart, scrollViewAtEnd } = this.state;
 
     return (
@@ -113,8 +145,8 @@ class NavList extends React.Component<NavListProps> {
                 {isHorizontal && (
                   <button
                     className={css(styles.navScrollButton)}
-                    aria-label={ariaLeftScroll}
-                    onClick={this.scrollLeft}
+                    aria-label={backScrollAriaLabel || ariaLeftScroll}
+                    onClick={this.scrollBack}
                     disabled={scrollViewAtStart}
                     tabIndex={isSidebarOpen ? null : -1}
                   >
@@ -133,8 +165,8 @@ class NavList extends React.Component<NavListProps> {
                 {isHorizontal && (
                   <button
                     className={css(styles.navScrollButton)}
-                    aria-label={ariaRightScroll}
-                    onClick={this.scrollRight}
+                    aria-label={forwardScrollAriaLabel || ariaRightScroll}
+                    onClick={this.scrollForward}
                     disabled={scrollViewAtEnd}
                     tabIndex={isSidebarOpen ? null : -1}
                   >

diff --git a/packages/react-core/src/components/Nav/__tests__/__snapshots__/Nav.test.tsx.snap b/packages/react-core/src/components/Nav/__tests__/__snapshots__/Nav.test.tsx.snap
@@ -573,7 +573,7 @@ exports[`Nav Horizontal Nav List 1`] = `
     data-ouia-safe="true"
   >
     <button
-      aria-label="Scroll left"
+      aria-label="Scroll back"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >
@@ -649,7 +649,7 @@ exports[`Nav Horizontal Nav List 1`] = `
       </li>
     </ul>
     <button
-      aria-label="Scroll right"
+      aria-label="Scroll foward"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >
@@ -681,7 +681,7 @@ exports[`Nav Horizontal SubNav List 1`] = `
     data-ouia-safe="true"
   >
     <button
-      aria-label="Scroll left"
+      aria-label="Scroll back"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >
@@ -757,7 +757,7 @@ exports[`Nav Horizontal SubNav List 1`] = `
       </li>
     </ul>
     <button
-      aria-label="Scroll right"
+      aria-label="Scroll foward"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >
@@ -948,7 +948,7 @@ exports[`Nav Nav List with custom item nodes 1`] = `
     data-ouia-safe="true"
   >
     <button
-      aria-label="Scroll left"
+      aria-label="Scroll back"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >
@@ -984,7 +984,7 @@ exports[`Nav Nav List with custom item nodes 1`] = `
       </li>
     </ul>
     <button
-      aria-label="Scroll right"
+      aria-label="Scroll foward"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >
@@ -1016,7 +1016,7 @@ exports[`Nav Nav List with flyout 1`] = `
     data-ouia-safe="true"
   >
     <button
-      aria-label="Scroll left"
+      aria-label="Scroll back"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >
@@ -1073,7 +1073,7 @@ exports[`Nav Nav List with flyout 1`] = `
       </li>
     </ul>
     <button
-      aria-label="Scroll right"
+      aria-label="Scroll foward"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >
@@ -1186,7 +1186,7 @@ exports[`Nav Tertiary Nav List 1`] = `
     data-ouia-safe="true"
   >
     <button
-      aria-label="Scroll left"
+      aria-label="Scroll back"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >
@@ -1262,7 +1262,7 @@ exports[`Nav Tertiary Nav List 1`] = `
       </li>
     </ul>
     <button
-      aria-label="Scroll right"
+      aria-label="Scroll foward"
       class="pf-v5-c-nav__scroll-button"
       disabled=""
     >

diff --git a/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap b/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap
@@ -602,7 +602,7 @@ exports[`Page Check page to verify grouped nav and breadcrumb - new components s
             data-ouia-safe="true"
           >
             <button
-              aria-label="Scroll left"
+              aria-label="Scroll back"
               class="pf-v5-c-nav__scroll-button"
               disabled=""
             >
@@ -687,7 +687,7 @@ exports[`Page Check page to verify grouped nav and breadcrumb - new components s
               </li>
             </ul>
             <button
-              aria-label="Scroll right"
+              aria-label="Scroll foward"
               class="pf-v5-c-nav__scroll-button"
               disabled=""
             >
@@ -785,7 +785,7 @@ exports[`Page Check page to verify grouped nav and breadcrumb - old / props synt
             data-ouia-safe="true"
           >
             <button
-              aria-label="Scroll left"
+              aria-label="Scroll back"
               class="pf-v5-c-nav__scroll-button"
               disabled=""
             >
@@ -870,7 +870,7 @@ exports[`Page Check page to verify grouped nav and breadcrumb - old / props synt
               </li>
             </ul>
             <button
-              aria-label="Scroll right"
+              aria-label="Scroll foward"
               class="pf-v5-c-nav__scroll-button"
               disabled=""
             >
@@ -1070,7 +1070,7 @@ exports[`Page Check page to verify nav is created - PageNavigation syntax 1`] =
           data-ouia-safe="true"
         >
           <button
-            aria-label="Scroll left"
+            aria-label="Scroll back"
             class="pf-v5-c-nav__scroll-button"
             disabled=""
           >
@@ -1155,7 +1155,7 @@ exports[`Page Check page to verify nav is created - PageNavigation syntax 1`] =
             </li>
           </ul>
           <button
-            aria-label="Scroll right"
+            aria-label="Scroll foward"
             class="pf-v5-c-nav__scroll-button"
             disabled=""
           >