fix(eye-icon): Change visibility eye icon to button (#765)

* wip move eye icon from table and change to button * wip show number of columns hidden * restrict button shrink to fit column text * fix column button size and wip visbility button test * wip mock onToggle * osFiltering rendering error * new test * mock state and onToggle function and clean up tests * remove old visibility code and test * add asChild prop to fix accessibility issue * address comments on PR wip * avoid min width on button * remove unused export * destructure props and make columns filter more explicit --------- Co-authored-by: Tiffany Vu <[email protected]> Co-authored-by: Tiffany Vu <[email protected]> Co-authored-by: 13bfrancis <[email protected]>
Enterprise-CMCS · Sep 16, 2024 · 45c528f · 45c528f
1 parent 6bad9c4
commit 45c528f
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 24 deletions.
diff --git a/react-app/src/components/Opensearch/main/Filtering/Filtering.test.tsx b/react-app/src/components/Opensearch/main/Filtering/Filtering.test.tsx
@@ -0,0 +1,69 @@
+import { useState } from "react";
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { describe, expect, test, beforeEach } from "vitest";
+import { type OsTableColumn, VisibilityPopover } from "../index";
+
+const MockOsFilteringWrapper = () => {
+  const [columns, setColumns] = useState<OsTableColumn[]>([
+    {
+      field: "state.keyword",
+      label: "State",
+      cell: () => "",
+    },
+    {
+      field: "authority.keyword",
+      label: "Authority",
+      cell: () => "",
+    },
+    {
+      field: "stateStatus.keyword",
+      label: "Status",
+      hidden: true,
+      cell: () => "",
+    },
+  ]);
+
+  const onToggle = (field: string) => {
+    setColumns((state) => {
+      return state?.map((S) => {
+        if (S.field !== field) return S;
+        return { ...S, hidden: !S.hidden };
+      });
+    });
+  };
+
+  return (
+    <VisibilityPopover
+      list={columns.filter((COL) => !COL.locked || COL.field)}
+      onItemClick={onToggle}
+      hiddenColumns={columns.filter((COL) => COL.hidden === true)}
+    />
+  );
+};
+
+describe("Visibility button", () => {
+  beforeEach(() => {
+    render(<MockOsFilteringWrapper />);
+  });
+
+  test("Visibility button should show number of hidden columns if any", async () => {
+    expect(screen.getByText("Columns (1 hidden)")).toBeInTheDocument();
+    await userEvent.click(screen.getByText("Columns (1 hidden)"));
+
+    const stateColumnMenuItem = screen.getByText("State");
+    await userEvent.click(stateColumnMenuItem);
+
+    expect(screen.getByText("Columns (2 hidden)")).toBeInTheDocument();
+  });
+
+  test("Visibility button text should not show number if no hidden columns", async () => {
+    expect(screen.getByText("Columns (1 hidden)")).toBeInTheDocument();
+    await userEvent.click(screen.getByText("Columns (1 hidden)"));
+
+    const statusColumnMenuItem = screen.getByText("Status");
+    await userEvent.click(statusColumnMenuItem);
+
+    expect(screen.getByText("Columns")).toBeInTheDocument();
+  });
+});
diff --git a/react-app/src/components/Opensearch/main/Filtering/index.tsx b/react-app/src/components/Opensearch/main/Filtering/index.tsx
@@ -4,11 +4,12 @@ import { useOsUrl } from "../useOpensearch";
 import { useOsContext } from "../Provider";
 import { OsFilterDrawer } from "./Drawer";
 import { OsExportData } from "./Export";
-
+import { VisibilityPopover } from "@/components";
 export const OsFiltering: FC<{
   columns: OsTableColumn[];
+  onToggle: (field: string) => void;
   disabled?: boolean;
-}> = (props) => {
+}> = ({ columns, onToggle, disabled }) => {
   const url = useOsUrl();
   const context = useOsContext();
 
@@ -27,11 +28,16 @@ export const OsFiltering: FC<{
               search,
             }))
           }
-          disabled={!!props.disabled}
+          disabled={!!disabled}
         />
         <div className="flex justify-center flex-row gap-2">
-          <OsExportData columns={props.columns} disabled={context?.data?.total.value === 0}/>
+          <VisibilityPopover
+            list={columns.filter((COL) => COL.locked === false || COL.field)}
+            onItemClick={onToggle}
+            hiddenColumns={columns.filter((COL) => COL.hidden === true)}
+          />
           <OsFilterDrawer />
+          <OsExportData columns={columns} disabled={context?.data?.total.value === 0}/>
         </div>
       </div>
     </div>

diff --git a/react-app/src/components/Opensearch/main/Settings/Visibility.tsx b/react-app/src/components/Opensearch/main/Settings/Visibility.tsx
@@ -7,28 +7,43 @@ import * as UI from "@/components";
 type Props<T extends UI.OsTableColumn> = {
   list: T[];
   onItemClick: (field: string) => void;
+  hiddenColumns: T[];
 };
 
-export const VisibilityPopover = <T extends UI.OsTableColumn>(
-  props: Props<T>
-) => {
+export const VisibilityPopover = ({
+  list,
+  onItemClick,
+  hiddenColumns,
+}: Props<UI.OsTableColumn>) => {
   return (
     <UI.Popover>
-      <UI.PopoverTrigger>
-        <EyeIcon className="w-6 h-6" />
-        <p className="sr-only">Visibility Popover Icon</p>
+      <UI.PopoverTrigger asChild>
+        <UI.Button
+          variant="outline"
+          className="w-full xs:w-fit min-w-0 whitespace-nowrap hover:bg-transparent self-center h-10 flex gap-2"
+        >
+          <span className="prose-sm">
+            {hiddenColumns.length
+              ? `Columns (${hiddenColumns.length} hidden)`
+              : "Columns"}
+          </span>
+        </UI.Button>
       </UI.PopoverTrigger>
       <UI.PopoverContent className="bg-white">
         <div className="flex flex-col gap-2">
-          <VisibilityMenu {...props} />
+          <VisibilityMenu
+            list={list}
+            onItemClick={onItemClick}
+            hiddenColumns={hiddenColumns}
+          />
         </div>
       </UI.PopoverContent>
     </UI.Popover>
   );
 };
 
 export const VisiblityItem = <T extends UI.OsTableColumn>(
-  props: T & { onClick: () => void }
+  props: T & { onClick: () => void },
 ) => {
   const eyeStyles = cn("flex flex-row gap-2 cursor-pointer", {
     "text-gray-800": !props.hidden,

diff --git a/react-app/src/components/Opensearch/main/Table/index.tsx b/react-app/src/components/Opensearch/main/Table/index.tsx
@@ -3,7 +3,6 @@ import type { FC } from "react";
 import { OsTableColumn } from "./types";
 import { useOsContext } from "../Provider";
 import { useOsUrl, LoadingSpinner } from "@/components";
-import { VisibilityPopover } from "../Settings";
 import { BLANK_VALUE } from "@/consts";
 import { opensearch } from "shared-types";
 
@@ -18,15 +17,6 @@ export const OsTable: FC<{
     <UI.Table className="overflow-scroll w-full">
       <UI.TableHeader className="sticky top-0 bg-white">
         <UI.TableRow>
-          <UI.TableHead
-            className="w-[10px]"
-            icon={
-              <VisibilityPopover
-                list={props.columns.filter((COL) => !COL.locked || COL.field)}
-                onItemClick={props.onToggle}
-              />
-            }
-          />
           {props.columns.map((TH) => {
             if (TH.hidden) return null;
             return (
@@ -79,7 +69,6 @@ export const OsTable: FC<{
         )}
         {context.data?.hits.map((DAT) => (
           <UI.TableRow className="max-h-1" key={DAT._source.id}>
-            <UI.TableCell className="fixed" />
             {props.columns.map((COL) => {
               if (COL.hidden) return null;
               return (

diff --git a/react-app/src/components/Opensearch/main/index.tsx b/react-app/src/components/Opensearch/main/index.tsx
@@ -31,7 +31,7 @@ export const OsMainView: FC<{
 
   return (
     <section className="flex md:items-stretch items-center flex-col h-[100vh]">
-      <OsFiltering columns={osColumns} />
+      <OsFiltering onToggle={onToggle} columns={osColumns}/>
       <OsTable onToggle={onToggle} columns={osColumns} />
       <Pagination
         pageNumber={url.state.pagination.number}