From bd4da1ef289ca76b6092eb93a456920ec2f3d6e5 Mon Sep 17 00:00:00 2001
From: Ben Hollis <ben@benhollis.net>
Date: Mon, 17 Feb 2025 16:02:33 -0800
Subject: [PATCH] Move compare headers into CompareItem (#10956)

---
 src/app/compare/Compare.m.scss          |  40 ---------
 src/app/compare/Compare.m.scss.d.ts     |   6 --
 src/app/compare/Compare.tsx             |  56 ++----------
 src/app/compare/CompareItem.m.scss      | 115 ++++++++++++++++++------
 src/app/compare/CompareItem.m.scss.d.ts |   6 ++
 src/app/compare/CompareItem.tsx         |  74 ++++++++++++++-
 src/app/organizer/ItemTable.m.scss      |   8 +-
 src/app/organizer/ItemTable.tsx         |  10 ++-
 8 files changed, 184 insertions(+), 131 deletions(-)

diff --git a/src/app/compare/Compare.m.scss b/src/app/compare/Compare.m.scss
index 7fa86d477a..4f9737e46b 100644
--- a/src/app/compare/Compare.m.scss
+++ b/src/app/compare/Compare.m.scss
@@ -1,30 +1,5 @@
 @use '../variables' as *;
 
-.statLabel {
-  position: relative;
-  padding-right: 8px;
-  padding-left: 4px;
-  cursor: pointer;
-  height: 16px;
-  img {
-    height: 16px;
-    width: 16px;
-    vertical-align: bottom;
-    margin-right: 2px;
-  }
-}
-
-.sortDesc {
-  color: var(--theme-accent-primary);
-}
-.sortAsc {
-  color: var(--theme-accent-secondary);
-}
-
-.spacer {
-  height: var(--compare-item-height, 48px);
-}
-
 .organizerLink {
   composes: dim-button from global;
   margin-left: auto;
@@ -34,11 +9,6 @@
   }
 }
 
-.statList {
-  display: flex;
-  flex-direction: column;
-}
-
 .bucket {
   white-space: nowrap;
   display: flex;
@@ -77,13 +47,3 @@
     }
   }
 }
-
-.highlightBar {
-  background-color: rgba(255, 255, 255, 0.125);
-  position: absolute;
-  top: 0;
-  left: 0;
-  height: 100%;
-  width: 100vw;
-  z-index: -1;
-}
diff --git a/src/app/compare/Compare.m.scss.d.ts b/src/app/compare/Compare.m.scss.d.ts
index da5223917c..9e54538439 100644
--- a/src/app/compare/Compare.m.scss.d.ts
+++ b/src/app/compare/Compare.m.scss.d.ts
@@ -2,14 +2,8 @@
 // Please do not change this file!
 interface CssExports {
   'bucket': string;
-  'highlightBar': string;
   'options': string;
   'organizerLink': string;
-  'sortAsc': string;
-  'sortDesc': string;
-  'spacer': string;
-  'statLabel': string;
-  'statList': string;
 }
 export const cssExports: CssExports;
 export default cssExports;
diff --git a/src/app/compare/Compare.tsx b/src/app/compare/Compare.tsx
index 0258fa6acd..4b24cf5ba9 100644
--- a/src/app/compare/Compare.tsx
+++ b/src/app/compare/Compare.tsx
@@ -1,4 +1,3 @@
-import BungieImage from 'app/dim-ui/BungieImage';
 import { SheetHorizontalScrollContainer } from 'app/dim-ui/SheetHorizontalScrollContainer';
 import { ColumnSort, SortDirection, useTableColumnSorts } from 'app/dim-ui/table-columns';
 import { t } from 'app/i18next-t';
@@ -12,17 +11,14 @@ import {
 import { recoilValue } from 'app/item-popup/RecoilStat';
 import { useD2Definitions } from 'app/manifest/selectors';
 import { showNotification } from 'app/notifications/notifications';
-import { statLabels } from 'app/organizer/Columns';
 import { weaponMasterworkY2SocketTypeHash } from 'app/search/d2-known-values';
 import Checkbox from 'app/settings/Checkbox';
 import { useSetting } from 'app/settings/hooks';
-import { AppIcon, faAngleLeft, faAngleRight, faList } from 'app/shell/icons';
+import { AppIcon, faList } from 'app/shell/icons';
 import { acquisitionRecencyComparator } from 'app/shell/item-comparators';
 import { useThunkDispatch } from 'app/store/thunk-dispatch';
 import { emptyArray } from 'app/utils/empty';
-import { useShiftHeld } from 'app/utils/hooks';
 import { DestinyDisplayPropertiesDefinition } from 'bungie-api-ts/destiny2';
-import clsx from 'clsx';
 import { StatHashes } from 'data/d2/generated-enums';
 import { maxBy } from 'es-toolkit';
 import React, { useCallback, useEffect, useMemo, useState } from 'react';
@@ -32,7 +28,7 @@ import Sheet from '../dim-ui/Sheet';
 import { DimItem, DimSocket, DimStat } from '../inventory/item-types';
 import { chainComparator, Comparator, compareBy, reverseComparator } from '../utils/comparators';
 import styles from './Compare.m.scss';
-import CompareItem from './CompareItem';
+import CompareItem, { CompareHeaders } from './CompareItem';
 import CompareSuggestions from './CompareSuggestions';
 import { endCompareSession, removeCompareItem, updateCompareQuery } from './actions';
 import { CompareSession } from './reducer';
@@ -207,50 +203,16 @@ export default function Compare({ session }: { session: CompareSession }) {
     </div>
   );
 
-  const isShiftHeld = useShiftHeld();
   return (
     <Sheet onClose={cancel} header={header} allowClickThrough>
       <div className={styles.bucket} onPointerLeave={() => setHighlight(undefined)}>
-        <div className={styles.statList}>
-          <div className={styles.spacer} />
-          {allStats.map((s) => {
-            const columnSort = columnSorts.find((c) => c.columnId === s.stat.statHash.toString());
-            return (
-              <div
-                key={s.stat.statHash}
-                className={clsx(
-                  styles.statLabel,
-                  columnSort
-                    ? columnSort.sort === SortDirection.ASC
-                      ? styles.sortDesc
-                      : styles.sortAsc
-                    : undefined,
-                )}
-                onPointerEnter={() => setHighlight(s.stat.statHash)}
-                onClick={toggleColumnSort(
-                  s.stat.statHash.toString(),
-                  isShiftHeld,
-                  s.stat.smallerIsBetter ? SortDirection.DESC : SortDirection.ASC,
-                )}
-              >
-                {s.stat.displayProperties.hasIcon && (
-                  <span title={s.stat.displayProperties.name}>
-                    <BungieImage src={s.stat.displayProperties.icon} />
-                  </span>
-                )}
-                {s.stat.statHash in statLabels
-                  ? t(statLabels[s.stat.statHash as StatHashes]!)
-                  : s.stat.displayProperties.name}{' '}
-                {columnSort && (
-                  <AppIcon
-                    icon={columnSort.sort === SortDirection.ASC ? faAngleRight : faAngleLeft}
-                  />
-                )}
-                {s.stat.statHash === highlight && <div className={styles.highlightBar} />}
-              </div>
-            );
-          })}
-        </div>
+        <CompareHeaders
+          columnSorts={columnSorts}
+          highlight={highlight}
+          setHighlight={setHighlight}
+          toggleColumnSort={toggleColumnSort}
+          allStats={allStats}
+        />
         {items}
       </div>
     </Sheet>
diff --git a/src/app/compare/CompareItem.m.scss b/src/app/compare/CompareItem.m.scss
index e7b9761641..d23ca9b29a 100644
--- a/src/app/compare/CompareItem.m.scss
+++ b/src/app/compare/CompareItem.m.scss
@@ -1,5 +1,50 @@
 @use '../variables.scss' as *;
 
+/* Row Headers */
+
+.spacer {
+  height: var(--compare-item-height, 48px);
+}
+
+.statList {
+  display: flex;
+  flex-direction: column;
+}
+
+.statLabel {
+  position: relative;
+  padding-right: 8px;
+  padding-left: 4px;
+  cursor: pointer;
+  height: 16px;
+  img {
+    height: 16px;
+    width: 16px;
+    vertical-align: bottom;
+    margin-right: 2px;
+  }
+}
+
+.sortDesc {
+  color: var(--theme-accent-primary);
+}
+.sortAsc {
+  color: var(--theme-accent-secondary);
+}
+
+// The highlight behind each item
+.highlightBar {
+  background-color: rgba(255, 255, 255, 0.125);
+  position: absolute;
+  top: 0;
+  left: 0;
+  height: 100%;
+  width: 100vw;
+  z-index: -1;
+}
+
+/* Items */
+
 .compareItem {
   composes: flexColumn from '../dim-ui/common.m.scss';
   border-left: 1px solid rgba(245, 245, 245, 0.25);
@@ -35,6 +80,7 @@
   }
 }
 
+// The toolbar across the top of each item
 .header {
   display: flex;
   flex-direction: row;
@@ -52,6 +98,31 @@
   }
 }
 
+// The "dismiss" button in the item header
+.close {
+  composes: resetButton from '../dim-ui/common.m.scss';
+  width: 32px;
+  height: 32px;
+  background-size: 16px;
+  background-image: url('images/close.png');
+  background-position: center;
+  background-repeat: no-repeat;
+
+  @include interactive($hover: true) {
+    background-color: var(--theme-accent-primary);
+  }
+}
+
+// The item icon that's floated behind the stats
+.itemAside {
+  position: absolute;
+  padding: 0;
+  right: 4px;
+  cursor: pointer;
+}
+
+/* COLUMN STYLES */
+
 .initialItem {
   color: var(--theme-accent-primary);
   font-weight: bold;
@@ -63,33 +134,25 @@
   overflow: hidden;
   margin-top: 2px;
   margin-bottom: 2px;
-  :global(.app-icon) {
-    font-size: 8px;
-    margin-right: 2px;
-    vertical-align: initial;
-  }
+  // Add the magnifying glass icons to items that can be clicked to find them
   &.isFindable {
     cursor: pointer;
-  }
-}
-
-.itemAside {
-  position: absolute;
-  padding: 0;
-  right: 4px;
-  cursor: pointer;
-}
-
-.close {
-  composes: resetButton from '../dim-ui/common.m.scss';
-  width: 32px;
-  height: 32px;
-  background-size: 16px;
-  background-image: url('images/close.png');
-  background-position: center;
-  background-repeat: no-repeat;
-
-  @include interactive($hover: true) {
-    background-color: var(--theme-accent-primary);
+    &::after {
+      content: '\f002';
+      // Copied from .fas classes:
+      /* stylelint-disable-next-line font-family-no-missing-generic-family-keyword */
+      font-family: 'Font Awesome 5 Free';
+      font-weight: 900;
+      -moz-osx-font-smoothing: grayscale;
+      -webkit-font-smoothing: antialiased;
+      display: inline-block;
+      font-style: normal;
+      font-variant: normal;
+      text-rendering: auto;
+      line-height: 1;
+      font-size: 8px;
+      margin-left: 4px;
+      vertical-align: initial;
+    }
   }
 }
diff --git a/src/app/compare/CompareItem.m.scss.d.ts b/src/app/compare/CompareItem.m.scss.d.ts
index 735a9e6fad..f9f889daea 100644
--- a/src/app/compare/CompareItem.m.scss.d.ts
+++ b/src/app/compare/CompareItem.m.scss.d.ts
@@ -4,10 +4,16 @@ interface CssExports {
   'close': string;
   'compareItem': string;
   'header': string;
+  'highlightBar': string;
   'initialItem': string;
   'isFindable': string;
   'itemAside': string;
   'itemName': string;
+  'sortAsc': string;
+  'sortDesc': string;
+  'spacer': string;
+  'statLabel': string;
+  'statList': string;
 }
 export const cssExports: CssExports;
 export default cssExports;
diff --git a/src/app/compare/CompareItem.tsx b/src/app/compare/CompareItem.tsx
index 015b38382d..42a8aca802 100644
--- a/src/app/compare/CompareItem.tsx
+++ b/src/app/compare/CompareItem.tsx
@@ -1,5 +1,7 @@
+import BungieImage from 'app/dim-ui/BungieImage';
 import { PressTip } from 'app/dim-ui/PressTip';
 import { useDynamicStringReplacer } from 'app/dim-ui/destiny-symbols/RichDestinyText';
+import { ColumnSort, SortDirection } from 'app/dim-ui/table-columns';
 import { t, tl } from 'app/i18next-t';
 import ItemPopupTrigger from 'app/inventory/ItemPopupTrigger';
 import { moveItemTo } from 'app/inventory/move-item';
@@ -7,11 +9,13 @@ import { currentStoreSelector, notesSelector } from 'app/inventory/selectors';
 import ActionButton from 'app/item-actions/ActionButton';
 import { LockActionButton, TagActionButton } from 'app/item-actions/ActionButtons';
 import { useD2Definitions } from 'app/manifest/selectors';
+import { statLabels } from 'app/organizer/Columns';
 import { useThunkDispatch } from 'app/store/thunk-dispatch';
 import { noop } from 'app/utils/functions';
-import { useSetCSSVarToHeight } from 'app/utils/hooks';
+import { useSetCSSVarToHeight, useShiftHeld } from 'app/utils/hooks';
 import { isD1Item } from 'app/utils/item-utils';
 import clsx from 'clsx';
+import { StatHashes } from 'data/d2/generated-enums';
 import { memo, useCallback, useMemo, useRef } from 'react';
 import { useSelector } from 'react-redux';
 import { useLocation } from 'react-router';
@@ -19,7 +23,13 @@ import ConnectedInventoryItem from '../inventory/ConnectedInventoryItem';
 import { DimItem, DimSocket } from '../inventory/item-types';
 import ItemSockets from '../item-popup/ItemSockets';
 import ItemTalentGrid from '../item-popup/ItemTalentGrid';
-import { AppIcon, faArrowCircleDown, searchIcon, shoppingCart } from '../shell/icons';
+import {
+  AppIcon,
+  faAngleLeft,
+  faAngleRight,
+  faArrowCircleDown,
+  shoppingCart,
+} from '../shell/icons';
 import { StatInfo } from './Compare';
 import styles from './CompareItem.m.scss';
 import CompareStat from './CompareStat';
@@ -77,8 +87,7 @@ export default memo(function CompareItem({
           })}
           onClick={() => itemClick(item)}
         >
-          <span title={isInitialItem ? t('Compare.InitialItem') : undefined}>{item.name}</span>{' '}
-          {isFindable && <AppIcon icon={searchIcon} />}
+          <span title={isInitialItem ? t('Compare.InitialItem') : undefined}>{item.name}</span>
         </div>
         <ItemPopupTrigger item={item} noCompare={true}>
           {(ref, onClick) => (
@@ -138,3 +147,60 @@ function VendorItemWarning({ item }: { item: DimItem }) {
     </PressTip>
   ) : null;
 }
+
+/** The row headers that appear on the left of the compare window */
+export function CompareHeaders({
+  columnSorts,
+  highlight,
+  setHighlight,
+  toggleColumnSort,
+  allStats,
+}: {
+  columnSorts: ColumnSort[];
+  highlight: string | number | undefined;
+  setHighlight: React.Dispatch<React.SetStateAction<string | number | undefined>>;
+  toggleColumnSort: (columnId: string, shiftHeld: boolean, sort?: SortDirection) => () => void;
+  allStats: StatInfo[];
+}) {
+  const isShiftHeld = useShiftHeld();
+  return (
+    <div className={styles.statList}>
+      <div className={styles.spacer} />
+      {allStats.map((s) => {
+        const columnSort = columnSorts.find((c) => c.columnId === s.stat.statHash.toString());
+        return (
+          <div
+            key={s.stat.statHash}
+            className={clsx(
+              styles.statLabel,
+              columnSort
+                ? columnSort.sort === SortDirection.ASC
+                  ? styles.sortDesc
+                  : styles.sortAsc
+                : undefined,
+            )}
+            onPointerEnter={() => setHighlight(s.stat.statHash)}
+            onClick={toggleColumnSort(
+              s.stat.statHash.toString(),
+              isShiftHeld,
+              s.stat.smallerIsBetter ? SortDirection.DESC : SortDirection.ASC,
+            )}
+          >
+            {s.stat.displayProperties.hasIcon && (
+              <span title={s.stat.displayProperties.name}>
+                <BungieImage src={s.stat.displayProperties.icon} />
+              </span>
+            )}
+            {s.stat.statHash in statLabels
+              ? t(statLabels[s.stat.statHash as StatHashes]!)
+              : s.stat.displayProperties.name}{' '}
+            {columnSort && (
+              <AppIcon icon={columnSort.sort === SortDirection.ASC ? faAngleRight : faAngleLeft} />
+            )}
+            {s.stat.statHash === highlight && <div className={styles.highlightBar} />}
+          </div>
+        );
+      })}
+    </div>
+  );
+}
diff --git a/src/app/organizer/ItemTable.m.scss b/src/app/organizer/ItemTable.m.scss
index 4b522c8128..48ecdf296c 100644
--- a/src/app/organizer/ItemTable.m.scss
+++ b/src/app/organizer/ItemTable.m.scss
@@ -21,6 +21,10 @@ $content-cells: 5;
     padding-top: calc(var(--item-size) * 0.75 * 0.5 - 4px);
     background-color: var(--theme-organizer-row-odd-bg);
   }
+
+  [role='cell'] {
+    z-index: $content-cells;
+  }
 }
 
 .toolbar {
@@ -76,10 +80,6 @@ $content-cells: 5;
   margin-left: 2px;
 }
 
-[role='cell'] {
-  z-index: $content-cells;
-}
-
 .selection {
   padding-left: 8px !important;
   padding-right: 2px !important;
diff --git a/src/app/organizer/ItemTable.tsx b/src/app/organizer/ItemTable.tsx
index 3ed2fb2b85..42b3614e9e 100644
--- a/src/app/organizer/ItemTable.tsx
+++ b/src/app/organizer/ItemTable.tsx
@@ -487,8 +487,10 @@ export default function ItemTable({ categories }: { categories: ItemCategoryTree
             />
           </div>
         </div>
-        {filteredColumns.map((column: ColumnDefinition) => {
-          const columnSort = columnSorts.find((c) => c.columnId === column.id);
+        {filteredColumns.map((column) => {
+          const columnSort = column.noSort
+            ? undefined
+            : columnSorts.find((c) => c.columnId === column.id);
           return (
             <div
               key={column.id}
@@ -510,7 +512,7 @@ export default function ItemTable({ categories }: { categories: ItemCategoryTree
                 }
               >
                 {column.header}
-                {!column.noSort && columnSort && (
+                {columnSort && (
                   <AppIcon
                     className={styles.sorter}
                     icon={columnSort.sort === SortDirection.DESC ? faCaretDown : faCaretUp}
@@ -600,7 +602,7 @@ function TableRow({
 }) {
   return (
     <>
-      {filteredColumns.map((column: ColumnDefinition) => (
+      {filteredColumns.map((column) => (
         // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
         <div
           key={column.id}