refactor: CellSetButton.tsx

chanzuckerberg · Oct 24, 2024 · 0cc317d · 0cc317d
1 parent c3dc2ed
commit 0cc317d
Show file tree

Hide file tree

Showing 9 changed files with 165 additions and 97 deletions.
diff --git a/.infra/rdev/values.yaml b/.infra/rdev/values.yaml
@@ -2,7 +2,7 @@ stack:
   services:
     explorer:
       image:
-        tag: sha-064d4b58
+        tag: sha-257b5270
       replicaCount: 1
       env:
         # env vars common to all deployment stages

diff --git a/client/src/components/DiffexNotice/style.ts b/client/src/components/DiffexNotice/style.ts
@@ -1,6 +1,6 @@
 import styled from "@emotion/styled";
 import Snackbar from "@mui/material/Snackbar";
-import { Alert } from "@mui/lab";
+import { Alert } from "@mui/material";
 
 export const StyledSnackbar = styled(Snackbar)`
   width: 423px;

diff --git a/client/src/components/MenuBar/components/DiffExpButtons/DiffExpButtons.tsx b/client/src/components/MenuBar/components/DiffExpButtons/DiffExpButtons.tsx
@@ -1,68 +1,38 @@
 import React from "react";
 import { connect } from "react-redux";
 import { ButtonGroup, AnchorButton, Tooltip } from "@blueprintjs/core";
-import actions from "actions";
-import { AppDispatch, RootState } from "reducers";
-import { DifferentialState } from "reducers/differential";
 
 import * as globals from "~/globals";
 // @ts-expect-error ts-migrate(2307) FIXME: Cannot find module './menubar.css' or its correspo... Remove this comment to see the full error message
 import styles from "../../menubar.css";
 import CellSetButton from "./components/CellSetButton/CellSetButton";
-
-export interface DiffexpButtonsProps {
-  differential: DifferentialState;
-  diffexpMayBeSlow: boolean;
-  diffexpCellcountMax: number;
-  dispatch: AppDispatch;
-}
-
-const mapStateToProps = (state: RootState) => ({
-  differential: state.differential,
-  diffexpMayBeSlow: state.config?.parameters?.["diffexp-may-be-slow"] ?? false,
-  diffexpCellcountMax: state.config?.limits?.diffexp_cellcount_max ?? 0,
-});
-
-const mapDispatchToProps = (dispatch: AppDispatch) => ({
-  dispatch,
-});
+import {
+  DiffexpButtonsProps,
+  mapDispatchToProps,
+  mapStateToProps,
+} from "./types";
+import { useConnect } from "./connect";
 
 function DiffexpButtons(props: DiffexpButtonsProps): JSX.Element {
-  const computeDiffExp = () => {
-    const { dispatch, differential } = props;
-    if (differential.celllist1 && differential.celllist2) {
-      dispatch(
-        actions.requestDifferentialExpression(
-          differential.celllist1,
-          differential.celllist2
-        )
-      ).catch((error) => {
-        console.error("Failed to request differential expression:", error);
-      });
-    }
-  };
+  const { dispatch, differential, diffexpMayBeSlow, diffexpCellcountMax } =
+    props;
+
+  const {
+    computeDiffExp,
+    warnMaxSizeExceeded,
+    tipMessageWarn,
+    haveBothCellSets,
+    tipMessage,
+  } = useConnect({
+    dispatch,
+    differential,
+    diffexpMayBeSlow,
+    diffexpCellcountMax,
+  });
 
-  const { differential, diffexpMayBeSlow, diffexpCellcountMax } = props;
-  const haveBothCellSets = !!differential.celllist1 && !!differential.celllist2;
-  const haveEitherCellSet =
-    !!differential.celllist1 || !!differential.celllist2;
-  const slowMsg = diffexpMayBeSlow
-    ? " (CAUTION: large dataset - may take longer or fail)"
-    : "";
-  const tipMessage = `See top differentially expressed genes${slowMsg}`;
-  const tipMessageWarn = `The total number of cells for differential expression computation
-                            may not exceed ${diffexpCellcountMax}. Try reselecting new cell sets.`;
-  const warnMaxSizeExceeded =
-    haveEitherCellSet &&
-    !!diffexpCellcountMax &&
-    (differential.celllist1?.length ?? 0) +
-      (differential.celllist2?.length ?? 0) >
-      diffexpCellcountMax;
   return (
     <ButtonGroup className={styles.menubarButton}>
-      {/* @ts-expect-error ts-migrate(2769) FIXME: No overload matches this call. */}
       <CellSetButton eitherCellSetOneOrTwo={1} />
-      {/* @ts-expect-error ts-migrate(2769) FIXME: No overload matches this call. */}
       <CellSetButton eitherCellSetOneOrTwo={2} />
       <Tooltip
         content={warnMaxSizeExceeded ? tipMessageWarn : tipMessage}

diff --git a/...c/components/MenuBar/components/DiffExpButtons/components/CellSetButton/CellSetButton.tsx b/...c/components/MenuBar/components/DiffExpButtons/components/CellSetButton/CellSetButton.tsx
@@ -1,55 +1,42 @@
 import React from "react";
 import { AnchorButton, Tooltip } from "@blueprintjs/core";
 import { connect } from "react-redux";
-import actions from "actions";
 import { track } from "analytics";
 import { EVENTS } from "analytics/events";
 import { tooltipHoverOpenDelay } from "~/globals";
+import {
+  CellSetButtonProps,
+  mapDispatchToProps,
+  mapStateToProps,
+} from "./types";
+import { useConnect } from "./connect";
 
-// @ts-expect-error ts-migrate(1238) FIXME: Unable to resolve signature of class decorator whe... Remove this comment to see the full error message
-@connect((state) => ({
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any --- FIXME: disabled temporarily on migrate to TS.
-  differential: (state as any).differential,
-}))
-class CellSetButton extends React.PureComponent {
-  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types --- FIXME: disabled temporarily on migrate to TS.
-  set() {
-    // @ts-expect-error ts-migrate(2339) FIXME: Property 'dispatch' does not exist on type 'Readon... Remove this comment to see the full error message
-    const { dispatch, eitherCellSetOneOrTwo } = this.props;
-    dispatch(actions.setCellSetFromSelection(eitherCellSetOneOrTwo));
-  }
+function CellSetButton(props: CellSetButtonProps) {
+  const { set, cellsSelected } = useConnect(props);
+  const { eitherCellSetOneOrTwo } = props;
 
-  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types --- FIXME: disabled temporarily on migrate to TS.
-  render() {
-    // @ts-expect-error ts-migrate(2339) FIXME: Property 'differential' does not exist on type 'Re... Remove this comment to see the full error message
-    const { differential, eitherCellSetOneOrTwo } = this.props;
-    const cellListName = `celllist${eitherCellSetOneOrTwo}`;
-    const cellsSelected = differential[cellListName]
-      ? differential[cellListName].length
-      : 0;
-    return (
-      <Tooltip
-        content="Save current selection for differential expression computation"
-        position="bottom"
-        hoverOpenDelay={tooltipHoverOpenDelay}
-      >
-        <AnchorButton
-          type="button"
-          onClick={() => {
-            track(EVENTS.EXPLORER_CELLSET_BUTTON_CLICKED);
+  return (
+    <Tooltip
+      content="Save current selection for differential expression computation"
+      position="bottom"
+      hoverOpenDelay={tooltipHoverOpenDelay}
+    >
+      <AnchorButton
+        type="button"
+        onClick={() => {
+          track(EVENTS.EXPLORER_CELLSET_BUTTON_CLICKED);
 
-            this.set();
-          }}
-          data-testid={`cellset-button-${eitherCellSetOneOrTwo}`}
-        >
-          <span data-testid={`cellset-count-${eitherCellSetOneOrTwo}`}>
-            {cellsSelected}
-          </span>
-          {" cells"}
-        </AnchorButton>
-      </Tooltip>
-    );
-  }
+          set();
+        }}
+        data-testid={`cellset-button-${eitherCellSetOneOrTwo}`}
+      >
+        <span data-testid={`cellset-count-${eitherCellSetOneOrTwo}`}>
+          {cellsSelected}
+        </span>
+        {" cells"}
+      </AnchorButton>
+    </Tooltip>
+  );
 }
 
-export default CellSetButton;
+export default connect(mapStateToProps, mapDispatchToProps)(CellSetButton);
diff --git a/client/src/components/MenuBar/components/DiffExpButtons/components/CellSetButton/connect.ts b/client/src/components/MenuBar/components/DiffExpButtons/components/CellSetButton/connect.ts
@@ -0,0 +1,22 @@
+import actions from "actions";
+import { DifferentialState } from "reducers/differential";
+import { CellSetButtonProps } from "./types";
+
+export function useConnect({
+  dispatch,
+  differential,
+  eitherCellSetOneOrTwo,
+}: CellSetButtonProps) {
+  function set() {
+    dispatch(actions.setCellSetFromSelection(eitherCellSetOneOrTwo));
+  }
+
+  const cellListName: keyof DifferentialState = `celllist${eitherCellSetOneOrTwo}`;
+
+  const cellsSelected = differential[cellListName]?.length ?? 0;
+
+  return {
+    set,
+    cellsSelected,
+  };
+}
diff --git a/client/src/components/MenuBar/components/DiffExpButtons/components/CellSetButton/types.ts b/client/src/components/MenuBar/components/DiffExpButtons/components/CellSetButton/types.ts
@@ -0,0 +1,16 @@
+import { AppDispatch, RootState } from "reducers";
+import { DifferentialState } from "reducers/differential";
+
+export interface CellSetButtonProps {
+  differential: DifferentialState;
+  eitherCellSetOneOrTwo: 1 | 2;
+  dispatch: AppDispatch;
+}
+
+export const mapStateToProps = (state: RootState) => ({
+  differential: state.differential,
+});
+
+export const mapDispatchToProps = (dispatch: AppDispatch) => ({
+  dispatch,
+});
diff --git a/client/src/components/MenuBar/components/DiffExpButtons/connect.ts b/client/src/components/MenuBar/components/DiffExpButtons/connect.ts
@@ -0,0 +1,48 @@
+import actions from "actions";
+import { DiffexpButtonsProps } from "./types";
+import {
+  DIFFEXP_SLOW_MSG,
+  genTipMessageWarn,
+  genTipMessage,
+} from "./constants";
+
+export function useConnect({
+  dispatch,
+  differential,
+  diffexpMayBeSlow,
+  diffexpCellcountMax,
+}: DiffexpButtonsProps) {
+  function computeDiffExp() {
+    if (differential.celllist1 && differential.celllist2) {
+      dispatch(
+        actions.requestDifferentialExpression(
+          differential.celllist1,
+          differential.celllist2
+        )
+      ).catch((error) => {
+        console.error("Failed to request differential expression:", error);
+      });
+    }
+  }
+
+  const haveBothCellSets = !!differential.celllist1 && !!differential.celllist2;
+  const haveEitherCellSet =
+    !!differential.celllist1 || !!differential.celllist2;
+  const slowMsg = diffexpMayBeSlow ? DIFFEXP_SLOW_MSG : "";
+  const tipMessage = genTipMessage(slowMsg);
+  const tipMessageWarn = genTipMessageWarn(diffexpCellcountMax);
+  const warnMaxSizeExceeded =
+    haveEitherCellSet &&
+    !!diffexpCellcountMax &&
+    (differential.celllist1?.length ?? 0) +
+      (differential.celllist2?.length ?? 0) >
+      diffexpCellcountMax;
+
+  return {
+    computeDiffExp,
+    haveBothCellSets,
+    warnMaxSizeExceeded,
+    tipMessage,
+    tipMessageWarn,
+  };
+}
diff --git a/client/src/components/MenuBar/components/DiffExpButtons/constants.ts b/client/src/components/MenuBar/components/DiffExpButtons/constants.ts
@@ -0,0 +1,6 @@
+export const DIFFEXP_SLOW_MSG =
+  "(CAUTION: large dataset - may take longer or fail)";
+export const genTipMessage = (slowMsg: string) =>
+  `See top differentially expressed genes${slowMsg}`;
+export const genTipMessageWarn = (cellCountMax: number) =>
+  `The total number of cells for differential expression computation may not exceed ${cellCountMax}. Try reselecting new cell sets.`;
diff --git a/client/src/components/MenuBar/components/DiffExpButtons/types.ts b/client/src/components/MenuBar/components/DiffExpButtons/types.ts
@@ -0,0 +1,19 @@
+import { AppDispatch, RootState } from "reducers";
+import { DifferentialState } from "reducers/differential";
+
+export interface DiffexpButtonsProps {
+  differential: DifferentialState;
+  diffexpMayBeSlow: boolean;
+  diffexpCellcountMax: number;
+  dispatch: AppDispatch;
+}
+
+export const mapStateToProps = (state: RootState) => ({
+  differential: state.differential,
+  diffexpMayBeSlow: state.config?.parameters?.["diffexp-may-be-slow"] ?? false,
+  diffexpCellcountMax: state.config?.limits?.diffexp_cellcount_max ?? 0,
+});
+
+export const mapDispatchToProps = (dispatch: AppDispatch) => ({
+  dispatch,
+});