Add filtering by undefined and fix searching by name (#2714)

* empty settings page and route * wow it was annoying to make the settings page * did not end up needing to change breadcrumbs * list providers * debug adding notifications in the backend, debug provider object class in the frontend, and allow for undefined values as - in filters * filter by name search was broken and tests were not behaving as expected * go mod tidy * filterByText formatting * filter dialog formatting * filter bugs and added filterSeparator to tests * removed an oopsie yaml file * one spot where filterSeparator was missing * bad fix for undefined chip label * SNAPS * better test for displaying dash in chip, chip group height * test changes * snapshot * remove override for chip text * test updates * remove screen.debug and make filterByText better * make search and filter optional on data table
weaveworks · Sep 12, 2022 · 7b7ceff · 7b7ceff
1 parent d0e288c
commit 7b7ceff
Show file tree

Hide file tree

Showing 8 changed files with 182 additions and 378 deletions.
diff --git a/ui/components/ChipGroup.tsx b/ui/components/ChipGroup.tsx
@@ -32,6 +32,7 @@ export default styled(ChipGroup).attrs({ className: ChipGroup.name })`
   .MuiChip-root {
     margin-right: ${(props) => props.theme.spacing.xxs};
   }
+  height: 40px;
   padding: 4px 0px;
   flex-wrap: nowrap;
   overflow-x: auto;

diff --git a/ui/components/DataTable.tsx b/ui/components/DataTable.tsx
@@ -161,31 +161,27 @@ function filterText(
 
   return _.filter(rows, (row) => {
     let matches = false;
-    for (const colName in row) {
-      const value = row[colName];
 
-      const field = _.find(fields, (f) => {
-        if (typeof f.value === "string") {
-          return f.value === colName;
-        }
+    fields.forEach((field) => {
+      if (!field.textSearchable) return matches;
 
-        if (f.sortValue) {
-          return f.sortValue(row) === value;
-        }
-      });
-
-      if (!field || !field.textSearchable) {
-        continue;
+      let value;
+      if (field.sortValue) {
+        value = field.sortValue(row);
+      } else {
+        value =
+          typeof field.value === "function"
+            ? field.value(row)
+            : row[field.value];
       }
 
-      // This allows us to look for a fragment in the string.
-      // For example, if the user searches for "pod", the "podinfo" kustomization should be returned.
-      for (const filterString of textFilters) {
-        if (_.includes(value, filterString)) {
-          matches = true;
+      for (let i = 0; i < textFilters.length; i++) {
+        matches = value.includes(textFilters[i]);
+        if (!matches) {
+          break;
         }
       }
-    }
+    });
 
     return matches;
   });
@@ -480,30 +476,37 @@ function UnstyledDataTable({
   });
   return (
     <Flex wide tall column className={className}>
-      <Flex wide align between>
+      <Flex
+        wide
+        align
+        between={filters ? true : false}
+        start={filters ? false : true}
+      >
         {checkboxes && <CheckboxActions checked={checked} rows={filtered} />}
-        <Flex wide align end>
-          <ChipGroup
-            chips={chips}
-            onChipRemove={handleChipRemove}
-            onClearAll={handleClearAll}
-          />
-          <IconFlex align>
-            <SearchField onSubmit={handleTextSearchSubmit} />
-            <IconButton
-              onClick={() => setFilterDialogOpen(!filterDialogOpen)}
-              className={className}
-              variant={filterDialogOpen ? "contained" : "text"}
-              color="inherit"
-            >
-              <Icon
-                type={IconType.FilterIcon}
-                size="medium"
-                color="neutral30"
-              />
-            </IconButton>
-          </IconFlex>
-        </Flex>
+        {filters && (
+          <Flex wide align end>
+            <ChipGroup
+              chips={chips}
+              onChipRemove={handleChipRemove}
+              onClearAll={handleClearAll}
+            />
+            <IconFlex align>
+              <SearchField onSubmit={handleTextSearchSubmit} />
+              <IconButton
+                onClick={() => setFilterDialogOpen(!filterDialogOpen)}
+                className={className}
+                variant={filterDialogOpen ? "contained" : "text"}
+                color="inherit"
+              >
+                <Icon
+                  type={IconType.FilterIcon}
+                  size="medium"
+                  color="neutral30"
+                />
+              </IconButton>
+            </IconFlex>
+          </Flex>
+        )}
       </Flex>
       <Flex wide tall>
         <TableContainer>

diff --git a/ui/components/FilterDialog.tsx b/ui/components/FilterDialog.tsx
@@ -33,7 +33,7 @@ const SlideContent = styled.div`
   padding-left: ${(props) => props.theme.spacing.large};
 `;
 
-export const filterSeparator = ":";
+export const filterSeparator = ": ";
 
 type FilterSectionProps = {
   header: string;
@@ -48,7 +48,9 @@ const FilterSection = ({
   formState,
   onSectionSelect,
 }: FilterSectionProps) => {
-  const compoundKeys = options.map((option) => `${header}:${option}`);
+  const compoundKeys = options.map(
+    (option) => `${header}${filterSeparator}${option}`
+  );
   // every on an empty list is true so check that too
   const all =
     compoundKeys.length > 0 && compoundKeys.every((key) => formState[key]);
@@ -77,21 +79,19 @@ const FilterSection = ({
             {convertHeaders(header)}
           </Text>
         </ListItem>
-        {options.sort().map((option: string, index: number) => {
-          return (
-            <ListItem key={index}>
-              <ListItemIcon>
-                <FormCheckbox
-                  label=""
-                  name={`${header}${filterSeparator}${option}`}
-                />
-              </ListItemIcon>
-              <Text color="neutral40" size="small">
-                {_.toString(option)}
-              </Text>
-            </ListItem>
-          );
-        })}
+        {options.sort().map((option: string, index: number) => (
+          <ListItem key={index}>
+            <ListItemIcon>
+              <FormCheckbox
+                label=""
+                name={`${header}${filterSeparator}${option}`}
+              />
+            </ListItemIcon>
+            <Text color="neutral40" size="small">
+              {_.toString(option) || "-"}
+            </Text>
+          </ListItem>
+        ))}
       </List>
     </ListItem>
   );

diff --git a/ui/components/__tests__/ChipGroup.test.tsx b/ui/components/__tests__/ChipGroup.test.tsx
@@ -6,7 +6,7 @@ import ChipGroup from "../ChipGroup";
 
 describe("ChipGroup", () => {
   const setActiveChips = jest.fn();
-  const chipList = ["app", "app2", "app3"];
+  const chipList = ["app", "app2", "app3", `appappapp`];
 
   it("should render chips", () => {
     render(

diff --git a/ui/components/__tests__/DataTable.test.tsx b/ui/components/__tests__/DataTable.test.tsx
@@ -103,7 +103,7 @@ describe("DataTable", () => {
       const firstRow = screen.getAllByRole("row")[1];
       expect(firstRow.innerHTML).toMatch(/podinfo/);
     });
-    it("should render text when rows is empty", () => {
+    it("should render text when rows are empty", () => {
       render(
         withTheme(
           withContext(