feat: Test navigation via URI fragment (#1188)

* Use test index as URI fragment with TestRun * Update test navigator to use correct URLs * Update test navigator * Handle tests that are test indexes beyond max * e2e test for clamping of the test index to the bounds of the test list * Try catch for ScrollFixer querySelector
w3c · Aug 6, 2024 · b2b0560 · b2b0560
1 parent 9013e81
commit b2b0560
Show file tree

Hide file tree

Showing 9 changed files with 160 additions and 81 deletions.
diff --git a/client/components/CandidateReview/CandidateTestPlanRun/index.jsx b/client/components/CandidateReview/CandidateTestPlanRun/index.jsx
@@ -30,6 +30,7 @@ import DisclosureComponent from '../../common/DisclosureComponent';
 import createIssueLink, {
   getIssueSearchLink
 } from '../../../utils/createIssueLink';
+import { useUrlTestIndex } from '../../../hooks/useUrlTestIndex';
 
 const CandidateTestPlanRun = () => {
   const { atId, testPlanVersionId } = useParams();
@@ -55,7 +56,8 @@ const CandidateTestPlanRun = () => {
   const [reviewStatus, setReviewStatus] = useState('');
   const [firstTimeViewing, setFirstTimeViewing] = useState(false);
   const [viewedTests, setViewedTests] = useState([]);
-  const [currentTestIndex, setCurrentTestIndex] = useState(0);
+  const [testsLength, setTestsLength] = useState(0);
+  const [currentTestIndex, setCurrentTestIndex] = useUrlTestIndex(testsLength);
   const [showTestNavigator, setShowTestNavigator] = useState(true);
   const [isFirstTest, setIsFirstTest] = useState(true);
   const [isLastTest, setIsLastTest] = useState(false);
@@ -181,7 +183,7 @@ const CandidateTestPlanRun = () => {
           return bools;
         });
       });
-
+      setTestsLength(tests.length);
       setShowBrowserClicks(browserClicks);
     }
   }, [data]);

diff --git a/client/components/TestRun/TestNavigator.jsx b/client/components/TestRun/TestNavigator.jsx
@@ -60,7 +60,7 @@ const TestNavigator = ({
           aria-labelledby="test-navigator-heading"
           className="test-navigator-list"
         >
-          {tests.map(test => {
+          {tests.map((test, index) => {
             let resultClassName = 'not-started';
             let resultStatus = 'Not Started';
 
@@ -126,8 +126,11 @@ const TestNavigator = ({
                 key={`TestNavigatorItem_${test.id}`}
               >
                 <a
-                  href="#"
-                  onClick={async () => await handleTestClick(test.index)}
+                  onClick={async e => {
+                    e.preventDefault();
+                    await handleTestClick(test.index);
+                  }}
+                  href={`#${index + 1}`}
                   className="test-name"
                   aria-current={test.index === currentTestIndex}
                 >

diff --git a/client/components/TestRun/index.jsx b/client/components/TestRun/index.jsx
@@ -1,4 +1,4 @@
-import React, { useEffect, useRef, useState } from 'react';
+import React, { useCallback, useEffect, useRef, useState } from 'react';
 import { Helmet } from 'react-helmet';
 import { Link, useNavigate, useParams } from 'react-router-dom';
 import useRouterQuery from '../../hooks/useRouterQuery';
@@ -41,6 +41,7 @@ import ReviewConflicts from '../ReviewConflicts';
 import createIssueLink from '../../utils/createIssueLink';
 import { dates } from 'shared';
 import { Provider as CollectionJobContextProvider } from './CollectionJobContext';
+import { useUrlTestIndex } from '../../hooks/useUrlTestIndex';
 
 const TestRun = () => {
   const params = useParams();
@@ -119,7 +120,7 @@ const TestRun = () => {
   const [testPlanReport, setTestPlanReport] = useState({});
   const [testPlanVersion, setTestPlanVersion] = useState();
   const [currentTest, setCurrentTest] = useState({});
-  const [currentTestIndex, setCurrentTestIndex] = useState(0);
+  const [currentTestIndex, setCurrentTestIndex] = useUrlTestIndex(tests.length);
   const [currentTestAtVersionId, setCurrentTestAtVersionId] = useState('');
   const [currentTestBrowserVersionId, setCurrentTestBrowserVersionId] =
     useState('');
@@ -144,6 +145,22 @@ const TestRun = () => {
   const isAdminReviewer = !!(isAdmin && openAsUserId);
   const openAsUser = users?.find(user => user.id === openAsUserId);
 
+  // Define createTestResultForRenderer as a memoized function
+  const createTestResultForRenderer = useCallback(
+    async (testId, atVersionId, browserVersionId) => {
+      const result = await createTestResult({
+        variables: {
+          testPlanRunId,
+          testId,
+          atVersionId,
+          browserVersionId
+        }
+      });
+      return result.data.testPlanRun.findOrCreateTestResult;
+    },
+    [createTestResult, testPlanRunId]
+  );
+
   useEffect(() => {
     reset();
 
@@ -169,7 +186,7 @@ const TestRun = () => {
         setPageReady(true);
       }
     } else if (data) setup(data);
-  }, [currentTestIndex]);
+  }, [currentTestIndex, createTestResultForRenderer]);
 
   const setup = data => {
     const { testPlanRun, users } = data;
@@ -313,22 +330,6 @@ const TestRun = () => {
 
   const toggleTestNavigator = () => setShowTestNavigator(!showTestNavigator);
 
-  const createTestResultForRenderer = async (
-    testId,
-    atVersionId,
-    browserVersionId
-  ) => {
-    const result = await createTestResult({
-      variables: {
-        testPlanRunId,
-        testId,
-        atVersionId,
-        browserVersionId
-      }
-    });
-    return result.data.testPlanRun.findOrCreateTestResult;
-  };
-
   // Check to see if there are tests to run
   const testCount = tests.length;
 

diff --git a/client/hooks/useUrlTestIndex.js b/client/hooks/useUrlTestIndex.js
@@ -0,0 +1,33 @@
+import { useLocation, useNavigate } from 'react-router-dom';
+import { useEffect, useState } from 'react';
+
+export const useUrlTestIndex = maxTestIndex => {
+  const location = useLocation();
+  const navigate = useNavigate();
+  const [currentTestIndex, setCurrentTestIndex] = useState(0);
+
+  const getTestIndex = () => {
+    // Remove the '#' character
+    const fragment = parseInt(location.hash.slice(1), 10) || 1;
+
+    if (!maxTestIndex || maxTestIndex < 1) {
+      // If the max test index is less than 1, return 0
+      return 0;
+    }
+    // Subtract 1 to make it 0-indexed
+    // and clamp to the max test index
+    return Math.max(0, Math.min(fragment - 1, maxTestIndex - 1));
+  };
+
+  useEffect(() => {
+    setCurrentTestIndex(getTestIndex());
+  }, [location.hash, maxTestIndex]);
+
+  const updateTestIndex = index => {
+    // Add 1 to make it 1-indexed
+    const newIndex = index + 1;
+    navigate(`${location.pathname}#${newIndex}`, { replace: true });
+  };
+
+  return [currentTestIndex, updateTestIndex];
+};
diff --git a/client/tests/e2e/TestRun.e2e.test.js b/client/tests/e2e/TestRun.e2e.test.js
@@ -55,6 +55,11 @@ describe('Test Run when not signed in', () => {
         await page.waitForSelector(`h2 ::-p-text(Instructions)`);
         await page.waitForSelector(`h2 ::-p-text(Record Results)`);
         await page.waitForSelector(`h3 ::-p-text(After)`);
+
+        const updatedUrl = await page.url();
+        expect(updatedUrl).toMatch(
+          new RegExp(`/test-plan-report/19#${index + 1}$`)
+        );
       }
 
       expect(h1Text.includes('Test 1:')).toBe(true);
@@ -174,6 +179,30 @@ describe('Test Run when signed in as tester', () => {
     });
   });
 
+  it('clamps the test index to the bounds of the test list', async () => {
+    await getPage({ role: 'tester', url: '/test-queue' }, async page => {
+      await assignSelfAndNavigateToRun(page);
+
+      await page.waitForSelector('h1 ::-p-text(Test 1)');
+      const url = await page.url();
+      await page.goto(`${url}#0`);
+      await page.waitForNetworkIdle();
+      let h1Text = await text(page, 'h1');
+      expect(h1Text).toMatch(/^Test 1:/);
+
+      const numberOfTests = await page.$eval(
+        'nav#test-navigator-nav ol',
+        el => {
+          return el.children.length;
+        }
+      );
+      await page.goto(`${url}#${numberOfTests + 10}`);
+      await page.waitForNetworkIdle();
+      h1Text = await text(page, 'h1');
+      expect(h1Text).toMatch(new RegExp(`Test ${numberOfTests}:`));
+    });
+  });
+
   it('inputs results and navigates between tests to confirm saving', async () => {
     async function getGeneratedCheckedTestCount(page, checkboxSelector) {
       return await page.$$eval(checkboxSelector, els => {

diff --git a/client/tests/e2e/snapshots/saved/_candidate-test-plan_24_1.html b/client/tests/e2e/snapshots/saved/_candidate-test-plan_24_1.html
@@ -116,133 +116,133 @@
                   aria-labelledby="test-navigator-heading"
                   class="test-navigator-list">
                   <li class="test-name-wrapper complete">
-                    <a href="#" class="test-name" aria-current="true"
+                    <a href="#1" class="test-name" aria-current="true"
                       >Open a Modal Dialog in reading mode</a
                     ><span
                       class="progress-indicator"
                       title="Test Viewed"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#2" class="test-name" aria-current="false"
                       >Open a Modal Dialog in interaction mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#3" class="test-name" aria-current="false"
                       >Close a modal dialog in reading mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#4" class="test-name" aria-current="false"
                       >Close a modal dialog in interaction mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#5" class="test-name" aria-current="false"
                       >Close a modal dialog using a button in reading mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#6" class="test-name" aria-current="false"
                       >Close a modal dialog using a button in interaction
                       mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#7" class="test-name" aria-current="false"
                       >Navigate to the last focusable element in a modal dialog
                       in interaction mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#8" class="test-name" aria-current="false"
                       >Navigate to the first focusable element in a modal dialog
                       in interaction mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#9" class="test-name" aria-current="false"
                       >Navigate to the beginning of a modal dialog in reading
                       mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#10" class="test-name" aria-current="false"
                       >Navigate to the end of a modal dialog in reading mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#11" class="test-name" aria-current="false"
                       >Open a nested modal dialog in reading mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#12" class="test-name" aria-current="false"
                       >Open a nested modal dialog in interaction mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#13" class="test-name" aria-current="false"
                       >Close a nested modal dialog in reading mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#14" class="test-name" aria-current="false"
                       >Close a nested modal dialog in interaction mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#15" class="test-name" aria-current="false"
                       >Close a nested modal dialog using a button in reading
                       mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#16" class="test-name" aria-current="false"
                       >Close a nested modal dialog using a button in interaction
                       mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#17" class="test-name" aria-current="false"
                       >Open a nested modal dialog using a link in reading
                       mode</a
                     ><span
                       class="progress-indicator"
                       title="Not Started"></span>
                   </li>
                   <li class="test-name-wrapper not-started">
-                    <a href="#" class="test-name" aria-current="false"
+                    <a href="#18" class="test-name" aria-current="false"
                       >Open a nested modal dialog using a link in interaction
                       mode</a
                     ><span