Small adjustments on the sort functionality (#837)

* Filter by the absolute value of delta instead of the signed value

* Make the sort direction at the first click 'desc' instead of 'asc'

src/__tests__/CompareResults/SubtestsResultsView.test.tsx

@@ -209,7 +209,7 @@ describe('SubtestsResultsView Component Tests', () => {
       // Initial view (alphabetical ordered, even if "sort by subtests" isn't specified
       expect(summarizeVisibleRows()).toEqual([
         'dhtml.html: 1.14 %, Low',
-        'improvement.html: 1.44 %, Low',
+        'improvement.html: -1.44 %, Low',
         'regression.html: 1.04 %, High',
         'tablemutation.html: 0.98 %, Low',
       ]);
@@ -219,102 +219,102 @@ describe('SubtestsResultsView Component Tests', () => {
       const deltaButton = screen.getByRole('button', { name: /Delta/ });
       expect(deltaButton).toMatchSnapshot();
       expect(window.location.search).not.toContain('sort=');
-      // Sort ascending
+      // Sort descending
       await user.click(deltaButton);
       expect(summarizeVisibleRows()).toEqual([
-        'tablemutation.html: 0.98 %, Low',
-        'regression.html: 1.04 %, High',
+        'improvement.html: -1.44 %, Low',
         'dhtml.html: 1.14 %, Low',
-        'improvement.html: 1.44 %, Low',
+        'regression.html: 1.04 %, High',
+        'tablemutation.html: 0.98 %, Low',
       ]);
-      // It should have the "ascending" SVG.
+      // It should have the "descending" SVG.
       expect(deltaButton).toMatchSnapshot();
       // It should be persisted in the URL
-      expectParameterToHaveValue('sort', 'delta|asc');
+      expectParameterToHaveValue('sort', 'delta|desc');
 
-      // Sort descending
+      // Sort ascending
       await user.click(deltaButton);
       expect(summarizeVisibleRows()).toEqual([
-        'improvement.html: 1.44 %, Low',
-        'dhtml.html: 1.14 %, Low',
-        'regression.html: 1.04 %, High',
         'tablemutation.html: 0.98 %, Low',
+        'regression.html: 1.04 %, High',
+        'dhtml.html: 1.14 %, Low',
+        'improvement.html: -1.44 %, Low',
       ]);
-      // It should have the "descending" SVG.
+      // It should have the "ascending" SVG.
       expect(deltaButton).toMatchSnapshot();
       // It should be persisted in the URL
-      expectParameterToHaveValue('sort', 'delta|desc');
+      expectParameterToHaveValue('sort', 'delta|asc');
 
-      // Sort by Confidence ascending
+      // Sort by Confidence descending
       const confidenceButton = screen.getByRole('button', {
         name: /Confidence.*sort/,
       });
       await user.click(confidenceButton);
       expect(summarizeVisibleRows()).toEqual([
-        'tablemutation.html: 0.98 %, Low',
-        'dhtml.html: 1.14 %, Low',
-        'improvement.html: 1.44 %, Low',
         'regression.html: 1.04 %, High',
+        'improvement.html: -1.44 %, Low',
+        'dhtml.html: 1.14 %, Low',
+        'tablemutation.html: 0.98 %, Low',
       ]);
       // It should have the "no sort" SVG.
       expect(deltaButton).toMatchSnapshot();
-      // It should have the "ascending" SVG.
+      // It should have the "descending" SVG.
       expect(confidenceButton).toMatchSnapshot();
       // It should be persisted in the URL
-      expectParameterToHaveValue('sort', 'confidence|asc');
+      expectParameterToHaveValue('sort', 'confidence|desc');
 
-      // Sort by subtest name ascending
+      // Sort by subtest name descending
       const subtestsButton = screen.getByRole('button', { name: /Subtests/ });
       await user.click(subtestsButton);
       expect(summarizeVisibleRows()).toEqual([
-        'dhtml.html: 1.14 %, Low',
-        'improvement.html: 1.44 %, Low',
-        'regression.html: 1.04 %, High',
         'tablemutation.html: 0.98 %, Low',
+        'regression.html: 1.04 %, High',
+        'improvement.html: -1.44 %, Low',
+        'dhtml.html: 1.14 %, Low',
       ]);
       // It should have the "no sort" SVG.
       expect(confidenceButton).toMatchSnapshot();
-      // It should have the "ascending" SVG.
+      // It should have the "descending" SVG.
       expect(subtestsButton).toMatchSnapshot();
       // It should be persisted in the URL
-      expectParameterToHaveValue('sort', 'subtests|asc');
+      expectParameterToHaveValue('sort', 'subtests|desc');
 
       // Clickince twice more should reset the URL.
       await user.click(subtestsButton);
       await user.click(subtestsButton);
       expect(window.location.search).not.toContain('sort=');
     });
 
-    it('initializes the sort from the URL at load time for an ascending sort', async () => {
+    it('initializes the sort from the URL at load time for a ascending sort', async () => {
       await setupForSorting({ extraParameters: 'sort=delta|asc' });
       await screen.findByText('dhtml.html');
       expect(summarizeVisibleRows()).toEqual([
         'tablemutation.html: 0.98 %, Low',
         'regression.html: 1.04 %, High',
         'dhtml.html: 1.14 %, Low',
-        'improvement.html: 1.44 %, Low',
+        'improvement.html: -1.44 %, Low',
       ]);
       // It should have the "ascending" SVG.
       expect(screen.getByRole('button', { name: /Delta/ })).toMatchSnapshot();
     });
 
-    it('initializes the sort from the URL at load time for an implicit ascending sort', async () => {
+    it('initializes the sort from the URL at load time for an implicit descending sort', async () => {
       await setupForSorting({ extraParameters: 'sort=delta' });
       await screen.findByText('dhtml.html');
       expect(summarizeVisibleRows()).toEqual([
-        'tablemutation.html: 0.98 %, Low',
-        'regression.html: 1.04 %, High',
+        'improvement.html: -1.44 %, Low',
         'dhtml.html: 1.14 %, Low',
-        'improvement.html: 1.44 %, Low',
+        'regression.html: 1.04 %, High',
+        'tablemutation.html: 0.98 %, Low',
       ]);
-      // It should have the "ascending" SVG.
+      // It should have the "descending" SVG.
       expect(screen.getByRole('button', { name: /Delta/ })).toMatchSnapshot();
     });
 
     it('initializes the sort from the URL at load time for a descending sort', async () => {
       await setupForSorting({ extraParameters: 'sort=delta|desc' });
       expect(summarizeVisibleRows()).toEqual([
-        'improvement.html: 1.44 %, Low',
+        'improvement.html: -1.44 %, Low',
         'dhtml.html: 1.14 %, Low',
         'regression.html: 1.04 %, High',
         'tablemutation.html: 0.98 %, Low',

src/__tests__/CompareResults/__snapshots__/SubtestsResultsView.test.tsx.snap

@@ -818,7 +818,7 @@ exports[`SubtestsResultsView Component Tests should render the subtests results
                   role="cell"
                 >
                    
-                  1.44
+                  -1.44
                    %
                    
                 </div>
@@ -1298,7 +1298,7 @@ exports[`SubtestsResultsView Component Tests table sorting can sort the table an
   type="button"
 >
   <svg
-    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-xzeo75-MuiSvgIcon-root"
+    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-1bpn2x6-MuiSvgIcon-root"
     data-testid="StraightIcon"
     focusable="false"
     role="img"
@@ -1308,7 +1308,7 @@ exports[`SubtestsResultsView Component Tests table sorting can sort the table an
       d="M11 6.83 9.41 8.41 8 7l4-4 4 4-1.41 1.41L13 6.83V21h-2z"
     />
     <title>
-      Sorted by Delta in ascending order
+      Sorted by Delta in descending order
     </title>
   </svg>
   Delta
@@ -1335,7 +1335,7 @@ exports[`SubtestsResultsView Component Tests table sorting can sort the table an
   type="button"
 >
   <svg
-    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-1bpn2x6-MuiSvgIcon-root"
+    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-xzeo75-MuiSvgIcon-root"
     data-testid="StraightIcon"
     focusable="false"
     role="img"
@@ -1345,7 +1345,7 @@ exports[`SubtestsResultsView Component Tests table sorting can sort the table an
       d="M11 6.83 9.41 8.41 8 7l4-4 4 4-1.41 1.41L13 6.83V21h-2z"
     />
     <title>
-      Sorted by Delta in descending order
+      Sorted by Delta in ascending order
     </title>
   </svg>
   Delta
@@ -1425,7 +1425,7 @@ exports[`SubtestsResultsView Component Tests table sorting can sort the table an
   type="button"
 >
   <svg
-    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-1mecf1h-MuiSvgIcon-root"
+    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-jmu928-MuiSvgIcon-root"
     data-testid="StraightIcon"
     focusable="false"
     role="img"
@@ -1435,7 +1435,7 @@ exports[`SubtestsResultsView Component Tests table sorting can sort the table an
       d="M11 6.83 9.41 8.41 8 7l4-4 4 4-1.41 1.41L13 6.83V21h-2z"
     />
     <title>
-      Sorted by Confidence in ascending order
+      Sorted by Confidence in descending order
     </title>
   </svg>
   <span
@@ -1497,7 +1497,7 @@ exports[`SubtestsResultsView Component Tests table sorting can sort the table an
   type="button"
 >
   <svg
-    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-xzeo75-MuiSvgIcon-root"
+    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-1bpn2x6-MuiSvgIcon-root"
     data-testid="StraightIcon"
     focusable="false"
     role="img"
@@ -1507,7 +1507,7 @@ exports[`SubtestsResultsView Component Tests table sorting can sort the table an
       d="M11 6.83 9.41 8.41 8 7l4-4 4 4-1.41 1.41L13 6.83V21h-2z"
     />
     <title>
-      Sorted by Subtests in ascending order
+      Sorted by Subtests in descending order
     </title>
   </svg>
   Subtests
@@ -1526,15 +1526,15 @@ exports[`SubtestsResultsView Component Tests table sorting can sort the table an
 </button>
 `;
 
-exports[`SubtestsResultsView Component Tests table sorting initializes the sort from the URL at load time for a descending sort 1`] = `
+exports[`SubtestsResultsView Component Tests table sorting initializes the sort from the URL at load time for a ascending sort 1`] = `
 <button
   aria-label="Delta (Currently sorted by this column. Click to change)"
   class="MuiButtonBase-root MuiButton-root MuiButton-contained MuiButton-containedTableHeaderButton MuiButton-sizeSmall MuiButton-containedSizeSmall MuiButton-disableElevation MuiButton-root MuiButton-contained MuiButton-containedTableHeaderButton MuiButton-sizeSmall MuiButton-containedSizeSmall MuiButton-disableElevation css-1awls4w-MuiButtonBase-root-MuiButton-root"
   tabindex="0"
   type="button"
 >
   <svg
-    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-1bpn2x6-MuiSvgIcon-root"
+    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-xzeo75-MuiSvgIcon-root"
     data-testid="StraightIcon"
     focusable="false"
     role="img"
@@ -1544,7 +1544,7 @@ exports[`SubtestsResultsView Component Tests table sorting initializes the sort
       d="M11 6.83 9.41 8.41 8 7l4-4 4 4-1.41 1.41L13 6.83V21h-2z"
     />
     <title>
-      Sorted by Delta in descending order
+      Sorted by Delta in ascending order
     </title>
   </svg>
   Delta
@@ -1554,15 +1554,15 @@ exports[`SubtestsResultsView Component Tests table sorting initializes the sort
 </button>
 `;
 
-exports[`SubtestsResultsView Component Tests table sorting initializes the sort from the URL at load time for an ascending sort 1`] = `
+exports[`SubtestsResultsView Component Tests table sorting initializes the sort from the URL at load time for a descending sort 1`] = `
 <button
   aria-label="Delta (Currently sorted by this column. Click to change)"
   class="MuiButtonBase-root MuiButton-root MuiButton-contained MuiButton-containedTableHeaderButton MuiButton-sizeSmall MuiButton-containedSizeSmall MuiButton-disableElevation MuiButton-root MuiButton-contained MuiButton-containedTableHeaderButton MuiButton-sizeSmall MuiButton-containedSizeSmall MuiButton-disableElevation css-1awls4w-MuiButtonBase-root-MuiButton-root"
   tabindex="0"
   type="button"
 >
   <svg
-    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-xzeo75-MuiSvgIcon-root"
+    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-1bpn2x6-MuiSvgIcon-root"
     data-testid="StraightIcon"
     focusable="false"
     role="img"
@@ -1572,7 +1572,7 @@ exports[`SubtestsResultsView Component Tests table sorting initializes the sort
       d="M11 6.83 9.41 8.41 8 7l4-4 4 4-1.41 1.41L13 6.83V21h-2z"
     />
     <title>
-      Sorted by Delta in ascending order
+      Sorted by Delta in descending order
     </title>
   </svg>
   Delta
@@ -1582,15 +1582,15 @@ exports[`SubtestsResultsView Component Tests table sorting initializes the sort
 </button>
 `;
 
-exports[`SubtestsResultsView Component Tests table sorting initializes the sort from the URL at load time for an implicit ascending sort 1`] = `
+exports[`SubtestsResultsView Component Tests table sorting initializes the sort from the URL at load time for an implicit descending sort 1`] = `
 <button
   aria-label="Delta (Currently sorted by this column. Click to change)"
   class="MuiButtonBase-root MuiButton-root MuiButton-contained MuiButton-containedTableHeaderButton MuiButton-sizeSmall MuiButton-containedSizeSmall MuiButton-disableElevation MuiButton-root MuiButton-contained MuiButton-containedTableHeaderButton MuiButton-sizeSmall MuiButton-containedSizeSmall MuiButton-disableElevation css-1awls4w-MuiButtonBase-root-MuiButton-root"
   tabindex="0"
   type="button"
 >
   <svg
-    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-xzeo75-MuiSvgIcon-root"
+    class="MuiSvgIcon-root MuiSvgIcon-fontSizeSmall css-1bpn2x6-MuiSvgIcon-root"
     data-testid="StraightIcon"
     focusable="false"
     role="img"
@@ -1600,7 +1600,7 @@ exports[`SubtestsResultsView Component Tests table sorting initializes the sort
       d="M11 6.83 9.41 8.41 8 7l4-4 4 4-1.41 1.41L13 6.83V21h-2z"
     />
     <title>
-      Sorted by Delta in ascending order
+      Sorted by Delta in descending order
     </title>
   </svg>
   Delta
@@ -2421,7 +2421,7 @@ exports[`SubtestsViewCompareOverTime Component Tests should render the subtests
                   role="cell"
                 >
                    
-                  1.44
+                  -1.44
                    %
                    
                 </div>

src/__tests__/utils/fixtures.ts

@@ -2478,8 +2478,8 @@ const getTestData = () => {
       confidence_text: 'Low',
       graphs_link:
         'https://treeherder.mozilla.org/perfherder/graphs?highlightedRevisions=d775409d7c6a&highlightedRevisions=22f4cf67e8ad&series=mozilla-central%2C5c47b31c38f7214a07fad2e0d41fb901cdc18eae%2C1%2C1&timerange=604800',
-      delta_value: 13.03,
-      delta_percentage: 1.44,
+      delta_value: -13.03,
+      delta_percentage: -1.44,
       magnitude: 5.68,
       new_is_better: false,
       lower_is_better: true,

src/components/CompareResults/SubtestsResults/SubtestsResultsTable.tsx

@@ -97,7 +97,9 @@ const columnsConfiguration: CompareResultsTableConfig = [
     key: 'delta',
     gridWidth: '1fr',
     sortFunction(resultA, resultB) {
-      return resultA.delta_percentage - resultB.delta_percentage;
+      return (
+        Math.abs(resultA.delta_percentage) - Math.abs(resultB.delta_percentage)
+      );
     },
   },
   {

src/components/CompareResults/TableHeader.tsx

@@ -199,14 +199,14 @@ function SortableColumnHeader({
   function onButtonClick() {
     let newSortDirection: typeof sortDirection;
     switch (sortDirection) {
-      case 'asc':
-        newSortDirection = 'desc';
-        break;
       case 'desc':
+        newSortDirection = 'asc';
+        break;
+      case 'asc':
         newSortDirection = null;
         break;
       default:
-        newSortDirection = 'asc';
+        newSortDirection = 'desc';
     }
 
     onToggle(newSortDirection);

src/hooks/useTableSort.ts

@@ -31,7 +31,7 @@ const useTableSort = (columnsConfiguration: CompareResultsTableConfig) => {
     }
 
     if (direction !== 'asc' && direction !== 'desc') {
-      return [columnId, 'asc'];
+      return [columnId, 'desc'];
     }
 
     return [columnId, direction];