Clean up notebook `SortStep` component (#29721)

* Move `SortStep` to its own directory

* Add tests for `SortStep`

* Clean up `SortStep`

* Improve a11y

frontend/src/metabase/query_builder/components/notebook/steps/SortStep.styled.tsx → frontend/src/metabase/query_builder/components/notebook/steps/SortStep/SortStep.styled.tsx

 import styled from "@emotion/styled";
 import { color } from "metabase/lib/colors";
+
 import FieldList from "metabase/query_builder/components/FieldList";
 
+export const SortDirectionButton = styled.button`
+  display: flex;
+  align-items: center;
+  gap: 0.5rem;
+
+  color: ${color("white")};
+  font-weight: 700;
+  cursor: pointer;
+`;
+
 export const SortFieldList = styled(FieldList)`
   color: ${color("summarize")};
 `;

frontend/src/metabase/query_builder/components/notebook/steps/SortStep.tsx → frontend/src/metabase/query_builder/components/notebook/steps/SortStep/SortStep.tsx

 import React from "react";
+import { t } from "ttag";
+
 import Icon from "metabase/components/Icon";
 
 import type {
@@ -9,59 +11,89 @@ import type DimensionOptions from "metabase-lib/DimensionOptions";
 import type StructuredQuery from "metabase-lib/queries/StructuredQuery";
 import type OrderBy from "metabase-lib/queries/structured/OrderBy";
 
-import type { NotebookStepUiComponentProps } from "../types";
-import ClauseStep from "./ClauseStep";
-import { SortFieldList } from "./SortStep.styled";
+import type { NotebookStepUiComponentProps } from "../../types";
+import ClauseStep from "../ClauseStep";
+import { SortDirectionButton, SortFieldList } from "./SortStep.styled";
 
 function SortStep({
-  color,
   query,
-  updateQuery,
+  color,
   isLastOpened,
   readOnly,
+  updateQuery,
 }: NotebookStepUiComponentProps) {
+  const handleUpdateSort = (sort: IOrderBy, index: number) => {
+    updateQuery(query.updateSort(index, sort));
+  };
+
+  const handleAddSort = (sort: IOrderBy) => {
+    updateQuery(query.sort(sort));
+  };
+
+  const handleRemoveSort = (sort: OrderBy, index: number) => {
+    return updateQuery(query.removeSort(index));
+  };
+
   return (
     <ClauseStep
       color={color}
       items={query.sorts()}
       readOnly={readOnly}
+      isLastOpened={isLastOpened}
       renderName={(sort, index) => (
-        <span
-          className="flex align-center"
-          onClick={e => {
-            e.stopPropagation();
-            updateQuery(
-              query.updateSort(index, [
-                sort[0] === "asc" ? "desc" : "asc",
-                sort[1],
-              ]),
-            );
-          }}
-        >
-          <Icon
-            name={sort[0] === "asc" ? "arrow_up" : "arrow_down"}
-            className="text-white mr1"
-          />
-          <span>{sort.dimension().displayName()}</span>
-        </span>
+        <SortDisplayName
+          sort={sort}
+          index={index}
+          onChange={handleUpdateSort}
+        />
       )}
       renderPopover={(sort, index) => (
         <SortPopover
           query={query}
           sort={sort}
-          onChangeSort={newSort =>
-            sort && typeof index === "number"
-              ? updateQuery(query.updateSort(index, newSort))
-              : updateQuery(query.sort(newSort))
-          }
+          onChangeSort={newSort => {
+            const isUpdate = sort && typeof index === "number";
+            return isUpdate
+              ? handleUpdateSort(newSort, index)
+              : handleAddSort(newSort);
+          }}
         />
       )}
-      isLastOpened={isLastOpened}
-      onRemove={(sort, index) => updateQuery(query.removeSort(index))}
+      onRemove={handleRemoveSort}
     />
   );
 }
 
+interface SortDisplayNameProps {
+  sort: OrderBy;
+  index: number;
+  onChange: (newSort: IOrderBy, index: number) => void;
+}
+
+function SortDisplayName({ sort, index, onChange }: SortDisplayNameProps) {
+  const [direction, fieldRef] = sort;
+
+  const displayName = sort.dimension().displayName();
+  const icon = direction === "asc" ? "arrow_up" : "arrow_down";
+
+  const handleToggleDirection = (event: React.MouseEvent<HTMLSpanElement>) => {
+    event.stopPropagation();
+    const nextDirection = direction === "asc" ? "desc" : "asc";
+    const nextSortClause: IOrderBy = [nextDirection, fieldRef];
+    onChange(nextSortClause, index);
+  };
+
+  return (
+    <SortDirectionButton
+      aria-label={t`Change direction`}
+      onClick={handleToggleDirection}
+    >
+      <Icon name={icon} />
+      <span>{displayName}</span>
+    </SortDirectionButton>
+  );
+}
+
 interface SortPopoverProps {
   sort?: OrderBy;
   query: StructuredQuery;
@@ -82,22 +114,27 @@ const SortPopover = ({
   alwaysExpanded,
 }: SortPopoverProps) => {
   const sort = sortProp || ["asc", null];
+  const [direction, field] = sort;
   const table = query.table();
 
+  const handleChangeField = (nextField: IField) => {
+    onChangeSort([direction, nextField]);
+    onClose?.();
+  };
+
   // FieldList requires table
   if (!table) {
     return null;
   }
 
+  const fieldOptions = sortOptions || query.sortOptions(field);
+
   return (
     <SortFieldList
       maxHeight={maxHeight}
-      field={sort && sort[1]}
-      fieldOptions={sortOptions || query.sortOptions(sort && sort[1])}
-      onFieldChange={(field: IField) => {
-        onChangeSort([sort[0], field]);
-        onClose?.();
-      }}
+      field={field}
+      fieldOptions={fieldOptions}
+      onFieldChange={handleChangeField}
       table={table}
       enableSubDimensions={false}
       useOriginalDimension={true}

frontend/src/metabase/query_builder/components/notebook/steps/SortStep/SortStep.unit.spec.tsx

+import React from "react";
+import userEvent from "@testing-library/user-event";
+import { render, screen, getIcon, queryIcon } from "__support__/ui";
+import { ORDERS, PRODUCTS } from "__support__/sample_database_fixture";
+import type StructuredQuery from "metabase-lib/queries/StructuredQuery";
+import { createMockNotebookStep, DEFAULT_LEGACY_QUERY } from "../../test-utils";
+import SortStep from "./SortStep";
+
+function setup(step = createMockNotebookStep()) {
+  const updateQuery = jest.fn();
+
+  render(
+    <SortStep
+      step={step}
+      query={step.query}
+      topLevelQuery={step.topLevelQuery}
+      color="brand"
+      isLastOpened={false}
+      reportTimezone="UTC"
+      updateQuery={updateQuery}
+    />,
+  );
+
+  function getNextQuery() {
+    const [lastCall] = updateQuery.mock.calls.slice(-1);
+    return lastCall[0] as StructuredQuery;
+  }
+
+  return { getNextQuery, updateQuery };
+}
+
+describe("SortStep", () => {
+  it("should render correctly without a sort", () => {
+    setup();
+    expect(getIcon("add")).toBeInTheDocument();
+    expect(queryIcon("arrow_up")).not.toBeInTheDocument();
+    expect(queryIcon("arrow_down")).not.toBeInTheDocument();
+  });
+
+  it("should render correctly with asc sort set", () => {
+    const [field] = ORDERS.fields;
+    const query = DEFAULT_LEGACY_QUERY.sort(["asc", field.reference()]);
+    setup(createMockNotebookStep({ query }));
+
+    expect(screen.getByText(field.displayName())).toBeInTheDocument();
+    expect(getIcon("arrow_up")).toBeInTheDocument();
+    expect(queryIcon("arrow_down")).not.toBeInTheDocument();
+  });
+
+  it("should render correctly with desc sort set", () => {
+    const [field] = ORDERS.fields;
+    const query = DEFAULT_LEGACY_QUERY.sort(["desc", field.reference()]);
+    setup(createMockNotebookStep({ query }));
+
+    expect(screen.getByText(field.displayName())).toBeInTheDocument();
+    expect(getIcon("arrow_down")).toBeInTheDocument();
+    expect(queryIcon("arrow_up")).not.toBeInTheDocument();
+  });
+
+  it("should display sortable columns", () => {
+    setup();
+
+    userEvent.click(getIcon("add"));
+
+    expect(screen.getByText(ORDERS.objectName())).toBeInTheDocument();
+    expect(screen.getByText(PRODUCTS.objectName())).toBeInTheDocument();
+    expect(screen.getByText("User")).toBeInTheDocument();
+    ORDERS.fields.forEach(field =>
+      expect(screen.getByText(field.displayName())).toBeInTheDocument(),
+    );
+  });
+
+  it("should add a sort", () => {
+    const { getNextQuery } = setup();
+
+    userEvent.click(getIcon("add"));
+    userEvent.click(screen.getByText("Created At"));
+
+    const [sort] = getNextQuery().sorts();
+    const [direction] = sort.raw();
+    expect(sort.dimension().displayName()).toBe("Created At");
+    expect(direction).toBe("asc");
+  });
+
+  it("should toggle sort direction", () => {
+    const [field] = ORDERS.fields;
+    const query = DEFAULT_LEGACY_QUERY.sort(["asc", field.reference()]);
+    const { getNextQuery } = setup(createMockNotebookStep({ query }));
+
+    userEvent.click(screen.getByLabelText("Change direction"));
+
+    const [sort] = getNextQuery().sorts();
+    const [direction] = sort.raw();
+    expect(direction).toBe("desc");
+  });
+
+  it("should change sorting field", () => {
+    const [field] = ORDERS.fields;
+    const query = DEFAULT_LEGACY_QUERY.sort(["asc", field.reference()]);
+    const { getNextQuery } = setup(createMockNotebookStep({ query }));
+
+    userEvent.click(screen.getByText(field.displayName()));
+    userEvent.click(screen.getByText("Created At"));
+
+    const [sort] = getNextQuery().sorts();
+    expect(sort.dimension().displayName()).toBe("Created At");
+  });
+
+  it("should remove sorting", () => {
+    const [field] = ORDERS.fields;
+    const query = DEFAULT_LEGACY_QUERY.sort(["asc", field.reference()]);
+    const { getNextQuery } = setup(createMockNotebookStep({ query }));
+
+    userEvent.click(getIcon("close"));
+
+    expect(getNextQuery().sorts()).toHaveLength(0);
+  });
+});

frontend/src/metabase/query_builder/components/notebook/steps/SortStep/index.ts

+export { default } from "./SortStep";