ref(tables): consolidate sortable header cells into one SortableHeaderCell

static/app/components/replays/virtualizedGrid/headerCell.tsx

@@ -3,7 +3,8 @@ import styled from '@emotion/styled';
 
 import {Tooltip} from '@sentry/scraps/tooltip';
 
-import {IconArrow, IconInfo} from 'sentry/icons';
+import {SortableHeaderCell} from 'sentry/components/tables/sortableHeaderCell';
+import {IconInfo} from 'sentry/icons';
 
 type BaseRecord = Record<string, unknown>;
 export interface SortConfig<RecordType extends BaseRecord> {
@@ -19,11 +20,12 @@ type Props<SortableRecord extends BaseRecord> = {
   sortConfig: SortConfig<SortableRecord>;
   style: CSSProperties;
   tooltipTitle: undefined | ReactNode;
-  ref?: React.Ref<HTMLButtonElement>;
 };
 
 const StyledIconInfo = styled(IconInfo)`
-  display: block;
+  margin-left: ${p => p.theme.space.xs};
+  margin-top: 1px;
+  vertical-align: text-top;
 `;
 
 function CatchClicks({children}: {children: ReactNode}) {
@@ -37,47 +39,34 @@ export function HeaderCell<T extends BaseRecord>({
   sortConfig,
   style,
   tooltipTitle,
-  ref,
 }: Props<T>) {
   return (
-    <HeaderButton style={style} onClick={() => handleSort(field)} ref={ref}>
+    <HeaderButton
+      direction={sortConfig.by === field ? (sortConfig.asc ? 'asc' : 'desc') : undefined}
+      onSort={() => handleSort(field)}
+      style={style}
+    >
       {label}
       {tooltipTitle ? (
         <Tooltip isHoverable title={<CatchClicks>{tooltipTitle}</CatchClicks>}>
           <StyledIconInfo size="xs" />
         </Tooltip>
       ) : null}
-      <IconArrow
-        variant="muted"
-        size="xs"
-        direction={sortConfig.by === field && !sortConfig.asc ? 'down' : 'up'}
-        style={{visibility: sortConfig.by === field ? 'visible' : 'hidden'}}
-      />
     </HeaderButton>
   );
 }
 
-const HeaderButton = styled('button')`
-  border: 0;
+const HeaderButton = styled(SortableHeaderCell)`
   border-bottom: 1px solid ${p => p.theme.tokens.border.primary};
   background: ${p => p.theme.tokens.background.secondary};
   color: ${p => p.theme.tokens.content.secondary};
 
   font-size: ${p => p.theme.font.size.sm};
   font-weight: ${p => p.theme.font.weight.sans.medium};
   line-height: 16px;
-  text-align: unset;
   text-transform: uppercase;
-  white-space: nowrap;
 
   width: 100%;
-  display: flex;
-  align-items: center;
-  justify-content: space-between;
   padding: ${p => p.theme.space.xs} ${p => p.theme.space.md} ${p => p.theme.space.xs}
     ${p => p.theme.space.lg};
-
-  svg {
-    margin-left: ${p => p.theme.space['2xs']};
-  }
 `;

static/app/components/tables/gridEditable/index.spec.tsx

@@ -0,0 +1,60 @@
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import type {GridColumnOrder} from 'sentry/components/tables/gridEditable';
+import {GridEditable} from 'sentry/components/tables/gridEditable';
+
+type Row = {count: number; name: string};
+
+const DATA: Row[] = [{name: 'first', count: 1}];
+
+const COLUMN_ORDER: Array<GridColumnOrder<keyof Row>> = [
+  {key: 'name', name: 'Name'},
+  {key: 'count', name: 'Count'},
+];
+
+describe('GridEditable', () => {
+  it('announces descending when a column is sorted descending', () => {
+    render(
+      <GridEditable
+        columnOrder={COLUMN_ORDER}
+        columnSortBy={[{key: 'count', order: 'desc'}]}
+        data={DATA}
+        grid={{}}
+      />
+    );
+
+    expect(screen.getByRole('columnheader', {name: 'Count'})).toHaveAttribute(
+      'aria-sort',
+      'descending'
+    );
+    expect(screen.getByRole('columnheader', {name: 'Name'})).not.toHaveAttribute(
+      'aria-sort'
+    );
+  });
+
+  it('announces ascending when a column is sorted ascending', () => {
+    render(
+      <GridEditable
+        columnOrder={COLUMN_ORDER}
+        columnSortBy={[{key: 'name', order: 'asc'}]}
+        data={DATA}
+        grid={{}}
+      />
+    );
+
+    expect(screen.getByRole('columnheader', {name: 'Name'})).toHaveAttribute(
+      'aria-sort',
+      'ascending'
+    );
+  });
+
+  it('announces no sort when the table is unsorted', () => {
+    render(
+      <GridEditable columnOrder={COLUMN_ORDER} columnSortBy={[]} data={DATA} grid={{}} />
+    );
+
+    expect(screen.getByRole('columnheader', {name: 'Count'})).not.toHaveAttribute(
+      'aria-sort'
+    );
+  });
+});

static/app/components/tables/gridEditable/index.tsx

@@ -6,6 +6,7 @@ import InteractionStateLayer from '@sentry/scraps/interactionStateLayer';
 import {GridEditableEmptyData} from 'sentry/components/tables/gridEditable/GridEditableEmptyData';
 import {GridEditableError} from 'sentry/components/tables/gridEditable/GridEditableError';
 import {GridEditableLoading} from 'sentry/components/tables/gridEditable/GridEditableLoading';
+import {getAriaSort} from 'sentry/components/tables/sortableHeaderCell';
 import {onRenderCallback, Profiler} from 'sentry/utils/performanceForSentry';
 
 import {
@@ -91,8 +92,8 @@ type GridEditableProps<
    * based on this 3 main props.
    *
    * - `columnOrder` determines the columns to show, from left to right
-   * - `columnSortBy` is not used at the moment, however it might be better to
-   *   move sorting into Grid for performance
+   * - `columnSortBy` tells each header cell which sort state to announce; the
+   *   sort itself is still performed by the parent component
    */
   title?: ReactNode;
 };
@@ -299,6 +300,9 @@ export function GridEditable<
           // 1 levels under GridHeadCell
           props.columnOrder.map((column, i) => (
             <GridHeadCell
+              aria-sort={getAriaSort(
+                props.columnSortBy.find(sort => sort.key === column.key)?.order
+              )}
               data-test-id="grid-head-cell"
               key={`${i}.${String(column.key)}`}
               isFirst={i === 0}

static/app/components/tables/gridEditable/sortLink.tsx

@@ -5,15 +5,14 @@ import type {LocationDescriptorObject} from 'history';
 import {Link} from '@sentry/scraps/link';
 
 import type {ColumnAlign} from 'sentry/components/tables/gridEditable';
+import type {SortDirection} from 'sentry/components/tables/sortableHeaderCell';
 import {IconArrow} from 'sentry/icons';
 import {useNavigate} from 'sentry/utils/useNavigate';
 
-export type ColumnSort = 'desc' | 'asc' | undefined;
-
 type Props = {
   align: ColumnAlign;
   canSort: boolean;
-  direction: ColumnSort;
+  direction: SortDirection | undefined;
   title: React.ReactNode;
   generateSortLink?: () => LocationDescriptorObject | undefined;
   onClick?: (e: React.MouseEvent<HTMLAnchorElement>) => void;

static/app/components/tables/simpleTable/index.spec.tsx

@@ -1,4 +1,4 @@
-import {render, screen, within} from 'sentry-test/reactTestingLibrary';
+import {render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary';
 
 import {SimpleTable} from 'sentry/components/tables/simpleTable';
 
@@ -38,4 +38,56 @@ describe('SimpleTable component', () => {
     expect(within(row2).getByRole('cell', {name: '4'})).toBeInTheDocument();
     expect(within(row2).getByRole('cell', {name: '5'})).toBeInTheDocument();
   });
+
+  it('announces the sort direction when a column is sorted', () => {
+    render(
+      <SimpleTable>
+        <SimpleTable.Header>
+          <SimpleTable.HeaderCell sort="asc" handleSortClick={jest.fn()}>
+            A
+          </SimpleTable.HeaderCell>
+          <SimpleTable.HeaderCell handleSortClick={jest.fn()}>B</SimpleTable.HeaderCell>
+        </SimpleTable.Header>
+      </SimpleTable>
+    );
+
+    expect(screen.getByRole('columnheader', {name: 'A'})).toHaveAttribute(
+      'aria-sort',
+      'ascending'
+    );
+    expect(screen.getByRole('columnheader', {name: 'B'})).not.toHaveAttribute(
+      'aria-sort'
+    );
+  });
+
+  it('keeps the interaction state layer a direct child of the header cell when sortable', () => {
+    render(
+      <SimpleTable>
+        <SimpleTable.Header>
+          <SimpleTable.HeaderCell handleSortClick={jest.fn()}>A</SimpleTable.HeaderCell>
+        </SimpleTable.Header>
+      </SimpleTable>
+    );
+
+    const header = screen.getByRole('columnheader', {name: 'A'});
+
+    expect(within(header).getByRole('presentation').parentElement).toBe(header);
+  });
+
+  it('sorts when a sortable header is clicked', async () => {
+    const handleSortClick = jest.fn();
+    render(
+      <SimpleTable>
+        <SimpleTable.Header>
+          <SimpleTable.HeaderCell handleSortClick={handleSortClick}>
+            A
+          </SimpleTable.HeaderCell>
+        </SimpleTable.Header>
+      </SimpleTable>
+    );
+
+    await userEvent.click(screen.getByRole('columnheader', {name: 'A'}));
+
+    expect(handleSortClick).toHaveBeenCalledTimes(1);
+  });
 });

static/app/components/tables/simpleTable/index.tsx

@@ -1,4 +1,5 @@
 import type {ComponentProps, HTMLAttributes, RefObject} from 'react';
+import {Fragment} from 'react';
 import {css} from '@emotion/react';
 import type {Theme} from '@emotion/react';
 import styled from '@emotion/styled';
@@ -7,7 +8,11 @@ import InteractionStateLayer from '@sentry/scraps/interactionStateLayer';
 import {Flex} from '@sentry/scraps/layout';
 
 import {Panel} from 'sentry/components/panels/panel';
-import {IconArrow} from 'sentry/icons';
+import {
+  getAriaSort,
+  SortableHeaderCell,
+  type SortDirection,
+} from 'sentry/components/tables/sortableHeaderCell';
 import {defined} from 'sentry/utils/defined';
 
 interface TableProps extends HTMLAttributes<HTMLDivElement> {
@@ -45,31 +50,23 @@ function HeaderCell({
   children?: React.ReactNode;
   divider?: boolean;
   handleSortClick?: () => void;
-  sort?: 'asc' | 'desc';
+  sort?: SortDirection;
 }) {
-  const isSorted = sort !== undefined;
-  const canSort = handleSortClick !== undefined;
-
   return (
     <ColumnHeaderCell
       {...props}
-      aria-sort={isSorted ? (sort === 'asc' ? 'ascending' : 'descending') : undefined}
-      isSorted={isSorted}
-      onClick={handleSortClick}
+      aria-sort={getAriaSort(sort)}
+      direction={sort}
+      onSort={handleSortClick}
+      overlays={
+        <Fragment>
+          {divider && <HeaderDivider />}
+          {handleSortClick && <InteractionStateLayer />}
+        </Fragment>
+      }
       role="columnheader"
-      as={canSort ? 'button' : 'div'}
     >
-      {divider && <HeaderDivider />}
-      {canSort && <InteractionStateLayer />}
-      <Flex align="center">{children}</Flex>
-      {isSorted && (
-        <SortIndicator
-          aria-hidden
-          size="xs"
-          direction={sort === 'asc' ? 'up' : 'down'}
-          isSorted={isSorted}
-        />
-      )}
+      {children}
     </ColumnHeaderCell>
   );
 }
@@ -148,22 +145,15 @@ const HeaderDivider = styled('div')`
   height: 14px;
 `;
 
-const ColumnHeaderCell = styled('div')<{isSorted?: boolean}>`
-  background: none;
+const ColumnHeaderCell = styled(SortableHeaderCell)`
   outline: none;
-  border: none;
   padding: 0 ${p => p.theme.space.xl};
-  text-transform: inherit;
   font-weight: ${p => p.theme.font.weight.sans.medium};
-  text-align: left;
   font-size: ${p => p.theme.font.size.md};
   color: ${p => p.theme.tokens.content.secondary};
 
   position: relative;
-  display: flex;
-  align-items: center;
   justify-content: space-between;
-  gap: ${p => p.theme.space.md};
   height: 100%;
 
   &:focus-visible {
@@ -176,11 +166,9 @@ const ColumnHeaderCell = styled('div')<{isSorted?: boolean}>`
     }
   }
 
-  ${p =>
-    p.isSorted &&
-    css`
-      color: ${p.theme.tokens.content.primary};
-    `}
+  &[aria-sort] {
+    color: ${p => p.theme.tokens.content.primary};
+  }
 `;
 
 const rowLinkStyle = (p: {theme: Theme}) => css`
@@ -198,18 +186,6 @@ const rowLinkStyle = (p: {theme: Theme}) => css`
   }
 `;
 
-const SortIndicator = styled(IconArrow, {
-  shouldForwardProp: prop => prop !== 'isSorted',
-})<{isSorted?: boolean}>`
-  visibility: hidden;
-
-  ${p =>
-    p.isSorted &&
-    css`
-      visibility: visible;
-    `}
-`;
-
 const StyledEmptyMessage = styled('div')`
   grid-column: 1 / -1;
   min-height: 200px;