Minor FilterableGroup, ValueData UX fixes

* ValueData showing empty ranges instead of "(any)" in some cases. * Use small "Tune" icon (like modifiers) for editable values rather than FilterList icon which is used for disabling groups/criteria. * Add apply button to FilterableGroup downdown. * Fix FilterableGroup hint ranges sometimes not updating. * Add tooltip when FilterableGroup select all button is disabled.
DataBiosphere · Oct 21, 2024 · ea311af · ea311af
1 parent 6c0c17b
commit ea311af
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 62 deletions.
diff --git a/ui/src/components/iconButton.tsx b/ui/src/components/iconButton.tsx
@@ -0,0 +1,18 @@
+import { Palette, Theme } from "@mui/material/styles";
+
+// Only allow the palette entries that are regular color palettes.
+export type ColorsFromPalette = Pick<Palette, "primary" | "warning" | "error">;
+
+// The current MUI version doesn't support "contained" IconButtons like Button
+// does so add reusable styling to emulate it.
+export function containedIconButtonSx(palette: keyof ColorsFromPalette) {
+  return {
+    "&.MuiIconButton-root": {
+      backgroundColor: (theme: Theme) => theme.palette[palette].main,
+      color: (theme: Theme) => theme.palette[palette].contrastText,
+    },
+    "&.MuiIconButton-root:hover": {
+      backgroundColor: (theme: Theme) => theme.palette[palette].dark,
+    },
+  };
+}
diff --git a/ui/src/criteria/attribute.tsx b/ui/src/criteria/attribute.tsx
@@ -25,6 +25,7 @@ import useSWRImmutable from "swr/immutable";
 import * as tanagraUnderlay from "tanagra-underlay/underlayConfig";
 import { base64ToBytes } from "util/base64";
 import { safeRegExp } from "util/safeRegExp";
+import { isValid } from "util/valid";
 
 type Selection = {
   value: DataValue;
@@ -59,8 +60,9 @@ interface Data {
         ? [{ value: true, name: attribute.name }]
         : [],
       dataRanges:
-        attribute?.displayHintRangeMin !== undefined &&
-        attribute?.displayHintRangeMax !== undefined
+        isValid(attribute) &&
+        isValid(attribute.displayHintRangeMin) &&
+        isValid(attribute.displayHintRangeMax)
           ? [
               {
                 id: generateId(),

diff --git a/ui/src/criteria/filterableGroup.tsx b/ui/src/criteria/filterableGroup.tsx
@@ -6,6 +6,7 @@ import IconButton from "@mui/material/IconButton";
 import Paper from "@mui/material/Paper";
 import Popover from "@mui/material/Popover";
 import TablePagination from "@mui/material/TablePagination";
+import Tooltip from "@mui/material/Tooltip";
 import Typography from "@mui/material/Typography";
 import { CriteriaPlugin, generateId, registerCriteriaPlugin } from "cohort";
 import Checkbox from "components/checkbox";
@@ -307,6 +308,11 @@ function FilterableGroupEdit(props: FilterableGroupEditProps) {
     [props.config.columns]
   );
 
+  const selectAllEnabled =
+    instancesState?.data?.[0]?.total &&
+    instancesState.data[0].total >= 25 &&
+    instancesState.data[0].total <= 10000;
+
   return (
     <GridBox
       sx={{
@@ -350,28 +356,35 @@ function FilterableGroupEdit(props: FilterableGroupEditProps) {
                     filters={filters}
                     setFilters={setFilters}
                   />
-                  <Button
-                    variant="outlined"
-                    disabled={
-                      instancesState.data[0].total < 25 ||
-                      instancesState.data[0].total > 10000
-                    }
-                    endIcon={<AddIcon />}
-                    onClick={() =>
-                      updateLocalCriteria((data) => {
-                        data.selected.push({
-                          id: generateId(),
-                          all: {
-                            query: searchState?.query ?? "",
-                            values: filters,
-                            exclusions: [],
-                          },
-                        });
-                      })
+                  <Tooltip
+                    title={
+                      !selectAllEnabled
+                        ? "Number of results must be between 25 and 10,000 to Select all. For results less than 25, selections must be made individually."
+                        : ""
                     }
                   >
-                    Select all
-                  </Button>
+                    <span>
+                      <Button
+                        variant="outlined"
+                        disabled={!selectAllEnabled}
+                        endIcon={<AddIcon />}
+                        onClick={() =>
+                          updateLocalCriteria((data) => {
+                            data.selected.push({
+                              id: generateId(),
+                              all: {
+                                query: searchState?.query ?? "",
+                                values: filters,
+                                exclusions: [],
+                              },
+                            });
+                          })
+                        }
+                      >
+                        Select all
+                      </Button>
+                    </span>
+                  </Tooltip>
                 </GridLayout>
                 {instancesState.data[currentPage] ? (
                   <ResultsPage
@@ -562,6 +575,13 @@ function FilterButton(props: FilterButtonProps) {
   const open = !!anchorEl;
   const id = open ? "filterableGroup-filters" : undefined;
 
+  const onClose = () => {
+    setAnchorEl(null);
+    if (isValid(filters)) {
+      props.setFilters(filters);
+    }
+  };
+
   return (
     <>
       <Button
@@ -579,25 +599,27 @@ function FilterButton(props: FilterButtonProps) {
         id={id}
         open={open}
         anchorEl={anchorEl}
-        onClose={() => {
-          setAnchorEl(null);
-          if (isValid(filters)) {
-            props.setFilters(filters);
-          }
-        }}
+        onClose={() => onClose()}
         anchorOrigin={{
           vertical: "bottom",
           horizontal: "left",
         }}
       >
         <GridBox sx={{ p: 2, width: "500px" }}>
-          <ValueDataEdit
-            hintEntity={props.entity}
-            hintData={props.hintData}
-            valueConfigs={props.config.valueConfigs}
-            valueData={filters ?? props.filters}
-            update={(filters) => setFilters(filters ?? [])}
-          />
+          <GridLayout rows spacing={2} height="auto">
+            <ValueDataEdit
+              hintEntity={props.entity}
+              hintData={props.hintData}
+              valueConfigs={props.config.valueConfigs}
+              valueData={filters ?? props.filters}
+              update={(filters) => setFilters(filters ?? [])}
+            />
+            <GridLayout cols colAlign="right">
+              <Button variant="contained" onClick={() => onClose()}>
+                Apply
+              </Button>
+            </GridLayout>
+          </GridLayout>
         </GridBox>
       </Popover>
     </>
@@ -754,7 +776,7 @@ function generateFilters(
   const operands: tanagra.Filter[] = [];
 
   if (query !== "") {
-    // TODO(tjennison): Consider how to make this configurable.
+    // TODO(BENCH-4370): Consider how to make this configurable.
     if (rsRE.test(query)) {
       operands.push({
         filterType: tanagra.FilterFilterTypeEnum.Attribute,

diff --git a/ui/src/criteria/valueData.tsx b/ui/src/criteria/valueData.tsx
@@ -1,14 +1,14 @@
-import FilterListIcon from "@mui/icons-material/FilterList";
-import FilterListOffIcon from "@mui/icons-material/FilterListOff";
+import TuneIcon from "@mui/icons-material/Tune";
 import Chip from "@mui/material/Chip";
 import Divider from "@mui/material/Divider";
 import FormControl from "@mui/material/FormControl";
+import IconButton from "@mui/material/IconButton";
 import MenuItem from "@mui/material/MenuItem";
 import OutlinedInput from "@mui/material/OutlinedInput";
 import Select, { SelectChangeEvent } from "@mui/material/Select";
 import Typography from "@mui/material/Typography";
-import Checkbox from "components/checkbox";
 import { HintDataSelect } from "components/hintDataSelect";
+import { containedIconButtonSx } from "components/iconButton";
 import Loading from "components/loading";
 import { DataRange, RangeSlider } from "components/rangeSlider";
 import { dataValueFromProto, HintData, protoFromDataValue } from "data/source";
@@ -95,6 +95,7 @@ export function ValueDataEdit(props: ValueDataEditProps) {
       hintEntity: props.hintEntity,
       relatedEntity: props.relatedEntity,
       key: props.hintKey,
+      hintData: props.hintData,
     },
     async (key) => {
       const hintData =
@@ -249,7 +250,7 @@ export function ValueDataEdit(props: ValueDataEditProps) {
   return (
     <GridLayout rows height="auto">
       {props.title ? (
-        <GridLayout cols rowAlign="middle">
+        <GridLayout cols spacing={0.5} rowAlign="middle">
           <Typography variant="body2">{props.title}</Typography>
           <Loading status={hintDataState} size="small">
             {hasHints ? (
@@ -262,16 +263,22 @@ export function ValueDataEdit(props: ValueDataEditProps) {
                   e.stopPropagation();
                 }}
               >
-                <Checkbox
-                  size="small"
-                  fontSize="inherit"
-                  checked={!!props.valueData}
-                  checkedIcon={<FilterListIcon />}
-                  uncheckedIcon={<FilterListOffIcon />}
-                  onChange={() =>
-                    props.update(props.valueData ? undefined : [ANY_VALUE_DATA])
-                  }
-                />
+                <Typography variant="body2">
+                  <IconButton
+                    sx={
+                      props.valueData
+                        ? containedIconButtonSx("primary")
+                        : undefined
+                    }
+                    onClick={() =>
+                      props.update(
+                        props.valueData ? undefined : [ANY_VALUE_DATA]
+                      )
+                    }
+                  >
+                    <TuneIcon fontSize="inherit" />
+                  </IconButton>
+                </Typography>
               </GridBox>
             ) : null}
           </Loading>

diff --git a/ui/src/overview.tsx b/ui/src/overview.tsx
@@ -29,6 +29,7 @@ import {
   useCohortContext,
 } from "cohortContext";
 import Empty from "components/empty";
+import { containedIconButtonSx } from "components/iconButton";
 import Loading from "components/loading";
 import { useMenu } from "components/menu";
 import { SaveStatus } from "components/saveStatus";
@@ -914,18 +915,7 @@ function ParticipantsGroup(props: {
                   <IconButton
                     sx={
                       props.group.disabled
-                        ? {
-                            "&.MuiIconButton-root": {
-                              backgroundColor: (theme) =>
-                                theme.palette.warning.main,
-                              color: (theme) =>
-                                theme.palette.warning.contrastText,
-                            },
-                            "&.MuiIconButton-root:hover": {
-                              backgroundColor: (theme) =>
-                                theme.palette.warning.dark,
-                            },
-                          }
+                        ? containedIconButtonSx("warning")
                         : undefined
                     }
                     onClick={() => {