fix(pager): correctly associate labels to input fields

Neither the numeric input for page number nor the page size select had a label that was programatically associated to the input and contained descriptive text. This is now fixed. fix #5284 and #5632
Sage · Dec 2, 2022 · 6412ddc · 6412ddc
1 parent 6dd3ede
commit 6412ddc
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 18 deletions.
diff --git a/src/components/pager/__internal__/pager-navigation.component.js b/src/components/pager/__internal__/pager-navigation.component.js
@@ -109,19 +109,19 @@ const PagerNavigation = ({
       {!hasOnePage && renderButtonsBeforeCount()}
       {showPageCount && (
         <StyledPagerNavInner>
-          <StyledPagerNoSelect>{l.pager.pageX()}</StyledPagerNoSelect>
-          <Label htmlFor={currentPageId}>
-            <NumberInput
-              value={currentPage.toString()}
-              data-element="current-page"
-              onChange={handleCurrentPageChange}
-              onBlur={handlePageInputChange}
-              id={currentPageId}
-              onKeyUp={(ev) =>
-                Events.isEnterKey(ev) ? handlePageInputChange(ev) : false
-              }
-            />
+          <Label inline htmlFor={currentPageId}>
+            <StyledPagerNoSelect>{l.pager.pageX()}</StyledPagerNoSelect>
           </Label>
+          <NumberInput
+            value={currentPage.toString()}
+            data-element="current-page"
+            onChange={handleCurrentPageChange}
+            onBlur={handlePageInputChange}
+            id={currentPageId}
+            onKeyUp={(ev) =>
+              Events.isEnterKey(ev) ? handlePageInputChange(ev) : false
+            }
+          />
           <StyledPagerNoSelect>{l.pager.ofY(pageCount)}</StyledPagerNoSelect>
         </StyledPagerNavInner>
       )}

diff --git a/src/components/pager/__internal__/pager-navigation.spec.js b/src/components/pager/__internal__/pager-navigation.spec.js
@@ -48,7 +48,7 @@ describe("Pager Navigation", () => {
     assertStyleMatch(
       {
         padding: "0",
-        margin: "8px 4px 0 4px",
+        margin: "4px",
         lineHeight: "26px",
         minHeight: "24px",
       },

diff --git a/src/components/pager/pager.component.js b/src/components/pager/pager.component.js
@@ -1,10 +1,11 @@
-import React, { useState, useCallback, useEffect } from "react";
+import React, { useState, useCallback, useEffect, useRef } from "react";
 import PropTypes from "prop-types";
 
 import { Select } from "../select";
 import PagerNavigation from "./__internal__/pager-navigation.component";
 import Option from "../select/option/option.component";
 import useLocale from "../../hooks/__internal__/useLocale";
+import createGuid from "../../__internal__/utils/helpers/guid";
 import {
   StyledPagerContainer,
   StyledPagerSizeOptions,
@@ -44,6 +45,9 @@ const Pager = ({
   const [currentPageSize, setCurrentPageSize] = useState(pageSize);
   const [value, setValue] = useState(pageSize);
 
+  const guid = useRef(createGuid());
+  const pageSizeSelectId = `Pager_size_selector_${guid.current}`;
+
   const getPageCount = useCallback(() => {
     if (Number(totalRecords) < 0 || Number.isNaN(Number(totalRecords))) {
       return 1;
@@ -138,6 +142,7 @@ const Pager = ({
           onBlur={() => setValue(currentPageSize)}
           onKeyDown={handleKeyDown}
           data-element="page-select"
+          id={pageSizeSelectId}
         >
           {pageSizeSelectionOptions.map((sizeOption) => (
             <Option
@@ -156,7 +161,9 @@ const Pager = ({
     return (
       showPageSizeSelection && (
         <StyledPagerSizeOptionsInner>
-          {showPageSizeLabelBefore && <span>{l.pager.show()}</span>}
+          {showPageSizeLabelBefore && (
+            <label htmlFor={pageSizeSelectId}>{l.pager.show()}</label>
+          )}
           {sizeSelector()}
           {showPageSizeLabelAfter && (
             <div>{l.pager.records(currentPageSize, false)}</div>

diff --git a/src/components/pager/pager.spec.js b/src/components/pager/pager.spec.js
@@ -268,7 +268,7 @@ describe("Pager", () => {
         wrapper.find(StyledPagerSizeOptionsInner).getDOMNode().firstChild
           .textContent
       ).toEqual(
-        wrapper.find(StyledPagerSizeOptionsInner).find("span").first().text()
+        wrapper.find(StyledPagerSizeOptionsInner).find("label").first().text()
       );
       expect(wrapper.find(StyledPagerSizeOptionsInner).exists()).toBeTruthy();
       expect(
@@ -308,7 +308,7 @@ describe("Pager", () => {
         wrapper.find(StyledPagerSizeOptionsInner).getDOMNode().firstChild
           .textContent
       ).toEqual(
-        wrapper.find(StyledPagerSizeOptionsInner).find("span").first().text()
+        wrapper.find(StyledPagerSizeOptionsInner).find("label").first().text()
       );
       expect(
         wrapper.find(StyledPagerSizeOptionsInner).getDOMNode().lastChild

diff --git a/src/components/pager/pager.style.js b/src/components/pager/pager.style.js
@@ -78,7 +78,7 @@ const StyledPagerNavigation = styled.div`
 
   && ${StyledInputPresentation} {
     padding: 0;
-    margin: 8px 4px 0 4px;
+    margin: 4px;
     height: 26px;
     line-height: 26px;
     min-height: 24px;
@@ -95,6 +95,7 @@ const StyledPagerNavInner = styled.div`
   display: flex;
   align-items: center;
   padding: 0 12px;
+  margin: 4px 0;
 `;
 
 const StyledPagerLinkStyles = styled(Link)`
@@ -106,6 +107,7 @@ const StyledPagerLinkStyles = styled(Link)`
 const StyledPagerNoSelect = styled.div`
   user-select: none;
   white-space: nowrap;
+  font-weight: normal;
 `;
 
 const StyledPagerSummary = styled.div`