From 0e11e89233d2cc8543c61ac34a906065c1cf1e9f Mon Sep 17 00:00:00 2001
From: Sebastian Rindom <skrindom@gmail.com>
Date: Wed, 9 Oct 2024 16:50:47 +0200
Subject: [PATCH] fix: hover states on filters and chip groups (#9511)

* fix: hover states on filters and chip groups

* fix: create FilterChip component
---
 .../common/chip-group/chip-group.tsx          |   2 +-
 .../data-table-filter/date-filter.tsx         |  78 ++-----------
 .../data-table-filter/filter-chip.tsx         |  97 ++++++++++++++++
 .../data-table-filter/number-filter.tsx       |  98 +++-------------
 .../data-table-filter/select-filter.tsx       | 108 +++---------------
 .../data-table-filter/string-filter.tsx       |  93 ++-------------
 .../location-service-zone-manage-areas.tsx    |   2 +-
 7 files changed, 146 insertions(+), 332 deletions(-)
 create mode 100644 packages/admin/dashboard/src/components/table/data-table/data-table-filter/filter-chip.tsx

diff --git a/packages/admin/dashboard/src/components/common/chip-group/chip-group.tsx b/packages/admin/dashboard/src/components/common/chip-group/chip-group.tsx
index fef85505db..48ed2ef3d8 100644
--- a/packages/admin/dashboard/src/components/common/chip-group/chip-group.tsx
+++ b/packages/admin/dashboard/src/components/common/chip-group/chip-group.tsx
@@ -76,7 +76,7 @@ const Chip = ({ index, className, children }: ChipProps) => {
   return (
     <li
       className={clx(
-        "bg-ui-bg-component shadow-borders-base flex items-center divide-x overflow-hidden rounded-md",
+        "bg-ui-bg-component shadow-borders-base flex items-stretch divide-x overflow-hidden rounded-md",
         {
           "bg-ui-bg-component": variant === "component",
           "bg-ui-bg-base-": variant === "base",
diff --git a/packages/admin/dashboard/src/components/table/data-table/data-table-filter/date-filter.tsx b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/date-filter.tsx
index 5c367f679d..46a1087312 100644
--- a/packages/admin/dashboard/src/components/table/data-table/data-table-filter/date-filter.tsx
+++ b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/date-filter.tsx
@@ -1,8 +1,8 @@
-import { EllipseMiniSolid, XMarkMini } from "@medusajs/icons"
+import { EllipseMiniSolid } from "@medusajs/icons"
 import { DatePicker, Text, clx } from "@medusajs/ui"
 import * as Popover from "@radix-ui/react-popover"
 import isEqual from "lodash/isEqual"
-import { MouseEvent, useMemo, useState } from "react"
+import { useMemo, useState } from "react"
 
 import { t } from "i18next"
 import { useTranslation } from "react-i18next"
@@ -10,6 +10,7 @@ import { useDate } from "../../../../hooks/use-date"
 import { useSelectedParams } from "../hooks"
 import { useDataTableFilterContext } from "./context"
 import { IFilter } from "./types"
+import FilterChip from "./filter-chip"
 
 type DateFilterProps = IFilter
 
@@ -96,6 +97,8 @@ export const DateFilter = ({
 
   const displayValue = getDisplayValueFromPresets() || getCustomDisplayValue()
 
+  const [previousValue, setPreviousValue] = useState<string | undefined>(displayValue)
+
   const handleRemove = () => {
     selectedParams.delete()
     removeFilter(key)
@@ -105,6 +108,7 @@ export const DateFilter = ({
 
   const handleOpenChange = (open: boolean) => {
     setOpen(open)
+    setPreviousValue(displayValue)
 
     if (timeoutId) {
       clearTimeout(timeoutId)
@@ -119,7 +123,8 @@ export const DateFilter = ({
 
   return (
     <Popover.Root modal open={open} onOpenChange={handleOpenChange}>
-      <DateDisplay
+      <FilterChip
+        hadPreviousValue={!!previousValue}
         label={label}
         value={displayValue}
         onRemove={handleRemove}
@@ -236,73 +241,6 @@ export const DateFilter = ({
   )
 }
 
-type DateDisplayProps = {
-  label: string
-  value?: string
-  readonly?: boolean
-  onRemove: () => void
-}
-
-const DateDisplay = ({
-  label,
-  value,
-  readonly,
-  onRemove,
-}: DateDisplayProps) => {
-  const handleRemove = (e: MouseEvent<HTMLButtonElement>) => {
-    e.stopPropagation()
-    onRemove()
-  }
-
-  return (
-    <Popover.Trigger
-      asChild
-      className={clx(
-        "bg-ui-bg-field transition-fg shadow-borders-base text-ui-fg-subtle flex cursor-pointer select-none items-center rounded-md",
-        {
-          "hover:bg-ui-bg-field-hover": !readonly,
-          "data-[state=open]:bg-ui-bg-field-hover": !readonly,
-        }
-      )}
-    >
-      <div>
-        <div
-          className={clx("flex items-center justify-center px-2 py-1", {
-            "border-r": !!value,
-          })}
-        >
-          <Text size="small" weight="plus" leading="compact">
-            {label}
-          </Text>
-        </div>
-        {value && (
-          <div className="flex items-center">
-            <div key={value} className="border-r p-1 px-2">
-              <Text size="small" weight="plus" leading="compact">
-                {value}
-              </Text>
-            </div>
-          </div>
-        )}
-        {!readonly && value && (
-          <div>
-            <button
-              onClick={handleRemove}
-              className={clx(
-                "text-ui-fg-muted transition-fg flex items-center justify-center p-1",
-                "hover:bg-ui-bg-subtle-hover",
-                "active:bg-ui-bg-subtle-pressed active:text-ui-fg-base"
-              )}
-            >
-              <XMarkMini />
-            </button>
-          </div>
-        )}
-      </div>
-    </Popover.Trigger>
-  )
-}
-
 const today = new Date()
 today.setHours(0, 0, 0, 0)
 
diff --git a/packages/admin/dashboard/src/components/table/data-table/data-table-filter/filter-chip.tsx b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/filter-chip.tsx
new file mode 100644
index 0000000000..8545555929
--- /dev/null
+++ b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/filter-chip.tsx
@@ -0,0 +1,97 @@
+import { XMarkMini } from "@medusajs/icons"
+import { Text, clx } from "@medusajs/ui"
+import { useTranslation } from "react-i18next"
+import { MouseEvent } from "react"
+import * as Popover from "@radix-ui/react-popover"
+
+export type FilterChipProps = {
+  hadPreviousValue?: boolean
+  label: string
+  value?: string
+  readonly?: boolean
+  hasOperator?: boolean
+  onRemove: () => void
+}
+
+const FilterChip = ({
+  hadPreviousValue,
+  label,
+  value,
+  readonly,
+  hasOperator,
+  onRemove,
+}: FilterChipProps) => {
+  const { t } = useTranslation()
+
+  const handleRemove = (e: MouseEvent<HTMLButtonElement>) => {
+    e.stopPropagation()
+    onRemove()
+  }
+
+  return (
+    <div
+      className="bg-ui-bg-field transition-fg shadow-borders-base text-ui-fg-subtle flex cursor-default select-none items-stretch overflow-hidden rounded-md"
+    >
+      {!hadPreviousValue && (
+        <Popover.Anchor />
+      )}
+      <div
+        className={clx(
+          "flex items-center justify-center whitespace-nowrap px-2 py-1",
+          {
+            "border-r": !!(value || hadPreviousValue),
+          }
+        )}
+      >
+        <Text size="small" weight="plus" leading="compact">
+          {label}
+        </Text>
+      </div>
+      <div className="flex w-full items-center overflow-hidden">
+        {hasOperator && !!(value || hadPreviousValue) && (
+          <div className="border-r p-1 px-2">
+            <Text
+              size="small"
+              weight="plus"
+              leading="compact"
+              className="text-ui-fg-muted"
+            >
+              {t("general.is")}
+            </Text>
+          </div>
+        )}
+        {!!(value || hadPreviousValue) && (
+          <Popover.Trigger asChild className={clx("flex-1 cursor-pointer overflow-hidden border-r p-1 px-2",
+            {
+              "hover:bg-ui-bg-field-hover": !readonly,
+              "data-[state=open]:bg-ui-bg-field-hover": !readonly,
+            }
+          )}>
+            <Text
+              size="small"
+              leading="compact"
+              weight="plus"
+              className="truncate text-nowrap"
+            >
+              {value || "\u00A0"}
+            </Text>
+          </Popover.Trigger>
+        )}
+      </div>
+      {!readonly && !!(value || hadPreviousValue) && (
+        <button
+          onClick={handleRemove}
+          className={clx(
+            "text-ui-fg-muted transition-fg flex items-center justify-center p-1",
+            "hover:bg-ui-bg-subtle-hover",
+            "active:bg-ui-bg-subtle-pressed active:text-ui-fg-base"
+          )}
+        >
+          <XMarkMini />
+        </button>
+      )}
+    </div>
+  )
+}
+
+export default FilterChip
diff --git a/packages/admin/dashboard/src/components/table/data-table/data-table-filter/number-filter.tsx b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/number-filter.tsx
index f725ece90c..66520a284f 100644
--- a/packages/admin/dashboard/src/components/table/data-table/data-table-filter/number-filter.tsx
+++ b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/number-filter.tsx
@@ -1,11 +1,10 @@
-import { EllipseMiniSolid, XMarkMini } from "@medusajs/icons"
-import { Input, Label, Text, clx } from "@medusajs/ui"
+import { EllipseMiniSolid } from "@medusajs/icons"
+import { Input, Label, clx } from "@medusajs/ui"
 import * as Popover from "@radix-ui/react-popover"
 import * as RadioGroup from "@radix-ui/react-radio-group"
 import { debounce } from "lodash"
 import {
   ChangeEvent,
-  MouseEvent,
   useCallback,
   useEffect,
   useState,
@@ -15,6 +14,8 @@ import { useTranslation } from "react-i18next"
 import { useSelectedParams } from "../hooks"
 import { useDataTableFilterContext } from "./context"
 import { IFilter } from "./types"
+import { TFunction } from "i18next"
+import FilterChip from "./filter-chip"
 
 type NumberFilterProps = IFilter
 
@@ -40,6 +41,9 @@ export const NumberFilter = ({
   })
 
   const currentValue = selectedParams.get()
+  const [previousValue, setPreviousValue] = useState<string[] | undefined>(
+    currentValue
+  )
 
   const [operator, setOperator] = useState<Comparison | undefined>(
     getOperator(currentValue)
@@ -99,6 +103,7 @@ export const NumberFilter = ({
 
   const handleOpenChange = (open: boolean) => {
     setOpen(open)
+    setPreviousValue(currentValue)
 
     if (timeoutId) {
       clearTimeout(timeoutId)
@@ -131,11 +136,16 @@ export const NumberFilter = ({
   const LT_KEY = `${key}-lt`
   const EQ_KEY = key
 
+  const displayValue = parseDisplayValue(currentValue, t)
+  const previousDisplayValue = parseDisplayValue(previousValue, t)
+
   return (
     <Popover.Root modal open={open} onOpenChange={handleOpenChange}>
-      <NumberDisplay
+      <FilterChip
+        hasOperator
+        hadPreviousValue={!!previousDisplayValue}
         label={label}
-        value={currentValue}
+        value={displayValue}
         onRemove={handleRemove}
         readonly={readonly}
       />
@@ -242,23 +252,7 @@ export const NumberFilter = ({
   )
 }
 
-const NumberDisplay = ({
-  label,
-  value,
-  readonly,
-  onRemove,
-}: {
-  label: string
-  value?: string[]
-  readonly?: boolean
-  onRemove: () => void
-}) => {
-  const { t } = useTranslation()
-  const handleRemove = (e: MouseEvent<HTMLButtonElement>) => {
-    e.stopPropagation()
-    onRemove()
-  }
-
+const parseDisplayValue = (value: string[] | null | undefined, t: TFunction) => {
   const parsed = JSON.parse(value?.join(",") || "{}")
   let displayValue = ""
 
@@ -283,65 +277,7 @@ const NumberDisplay = ({
     displayValue = parsed.toString()
   }
 
-  return (
-    <Popover.Trigger
-      asChild
-      className={clx(
-        "bg-ui-bg-field transition-fg shadow-borders-base text-ui-fg-subtle flex cursor-pointer select-none items-center rounded-md",
-        {
-          "hover:bg-ui-bg-field-hover": !readonly,
-          "data-[state=open]:bg-ui-bg-field-hover": !readonly,
-        }
-      )}
-    >
-      <div>
-        <div
-          className={clx("flex items-center justify-center px-2 py-1", {
-            "border-r": !!value,
-          })}
-        >
-          <Text size="small" weight="plus" leading="compact">
-            {label}
-          </Text>
-        </div>
-        {!!value && (
-          <div className="border-r p-1 px-2">
-            <Text
-              size="small"
-              weight="plus"
-              leading="compact"
-              className="text-ui-fg-muted"
-            >
-              {t("general.is")}
-            </Text>
-          </div>
-        )}
-        {value && (
-          <div className="flex items-center">
-            <div className="border-r p-1 px-2">
-              <Text size="small" weight="plus" leading="compact">
-                {displayValue}
-              </Text>
-            </div>
-          </div>
-        )}
-        {!readonly && value && (
-          <div>
-            <button
-              onClick={handleRemove}
-              className={clx(
-                "text-ui-fg-muted transition-fg flex items-center justify-center p-1",
-                "hover:bg-ui-bg-subtle-hover",
-                "active:bg-ui-bg-subtle-pressed active:text-ui-fg-base"
-              )}
-            >
-              <XMarkMini />
-            </button>
-          </div>
-        )}
-      </div>
-    </Popover.Trigger>
-  )
+  return displayValue
 }
 
 const parseValue = (value: string[] | null | undefined) => {
diff --git a/packages/admin/dashboard/src/components/table/data-table/data-table-filter/select-filter.tsx b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/select-filter.tsx
index 64e4bc19cf..9244823e20 100644
--- a/packages/admin/dashboard/src/components/table/data-table/data-table-filter/select-filter.tsx
+++ b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/select-filter.tsx
@@ -1,13 +1,14 @@
 import { CheckMini, EllipseMiniSolid, XMarkMini } from "@medusajs/icons"
-import { Text, clx } from "@medusajs/ui"
+import { clx } from "@medusajs/ui"
 import * as Popover from "@radix-ui/react-popover"
 import { Command } from "cmdk"
-import { MouseEvent, useState } from "react"
+import { useState } from "react"
 import { useTranslation } from "react-i18next"
 
 import { useSelectedParams } from "../hooks"
 import { useDataTableFilterContext } from "./context"
 import { IFilter } from "./types"
+import FilterChip from "./filter-chip"
 
 interface SelectFilterProps extends IFilter {
   options: { label: string; value: unknown }[]
@@ -40,6 +41,8 @@ export const SelectFilter = ({
     .map((v) => options.find((o) => o.value === v)?.label)
     .filter(Boolean) as string[]
 
+  const [previousValue, setPreviousValue] = useState<string | string[] | undefined>(labelValues)
+
   const handleRemove = () => {
     selectedParams.delete()
     removeFilter(key)
@@ -50,6 +53,8 @@ export const SelectFilter = ({
   const handleOpenChange = (open: boolean) => {
     setOpen(open)
 
+    setPreviousValue(labelValues)
+
     if (timeoutId) {
       clearTimeout(timeoutId)
     }
@@ -79,12 +84,17 @@ export const SelectFilter = ({
     }
   }
 
+  const normalizedValues = labelValues ? (Array.isArray(labelValues) ? labelValues : [labelValues]) : null
+  const normalizedPrev = previousValue ? (Array.isArray(previousValue) ? previousValue : [previousValue]) : null
+
   return (
     <Popover.Root modal open={open} onOpenChange={handleOpenChange}>
-      <SelectDisplay
+      <FilterChip
+        hasOperator
+        hadPreviousValue={!!normalizedPrev?.length}
         readonly={readonly}
         label={label}
-        value={labelValues}
+        value={normalizedValues?.join(", ")}
         onRemove={handleRemove}
       />
       {!readonly && (
@@ -179,93 +189,3 @@ export const SelectFilter = ({
     </Popover.Root>
   )
 }
-
-type SelectDisplayProps = {
-  label: string
-  readonly?: boolean
-  value?: string | string[]
-  onRemove: () => void
-}
-
-export const SelectDisplay = ({
-  label,
-  value,
-  onRemove,
-  readonly,
-}: SelectDisplayProps) => {
-  const { t } = useTranslation()
-  const v = value ? (Array.isArray(value) ? value : [value]) : null
-  const count = v?.length || 0
-
-  const handleRemove = (e: MouseEvent<HTMLButtonElement>) => {
-    e.stopPropagation()
-    onRemove()
-  }
-
-  return (
-    <Popover.Trigger asChild>
-      <div
-        className={clx(
-          "bg-ui-bg-field transition-fg shadow-borders-base text-ui-fg-subtle flex cursor-pointer select-none items-center overflow-hidden rounded-md",
-          {
-            "hover:bg-ui-bg-field-hover": !readonly,
-            "data-[state=open]:bg-ui-bg-field-hover": !readonly,
-          }
-        )}
-      >
-        <div
-          className={clx(
-            "flex items-center justify-center whitespace-nowrap px-2 py-1",
-            {
-              "border-r": count > 0,
-            }
-          )}
-        >
-          <Text size="small" weight="plus" leading="compact">
-            {label}
-          </Text>
-        </div>
-        <div className="flex w-full items-center overflow-hidden">
-          {count > 0 && (
-            <div className="border-r p-1 px-2">
-              <Text
-                size="small"
-                weight="plus"
-                leading="compact"
-                className="text-ui-fg-muted"
-              >
-                {t("general.is")}
-              </Text>
-            </div>
-          )}
-          {count > 0 && (
-            <div className="flex-1 overflow-hidden border-r p-1 px-2">
-              <Text
-                size="small"
-                leading="compact"
-                weight="plus"
-                className="truncate text-nowrap"
-              >
-                {v?.join(", ")}
-              </Text>
-            </div>
-          )}
-        </div>
-        {!readonly && v && v.length > 0 && (
-          <div>
-            <button
-              onClick={handleRemove}
-              className={clx(
-                "text-ui-fg-muted transition-fg flex items-center justify-center p-1",
-                "hover:bg-ui-bg-subtle-hover",
-                "active:bg-ui-bg-subtle-pressed active:text-ui-fg-base"
-              )}
-            >
-              <XMarkMini />
-            </button>
-          </div>
-        )}
-      </div>
-    </Popover.Trigger>
-  )
-}
diff --git a/packages/admin/dashboard/src/components/table/data-table/data-table-filter/string-filter.tsx b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/string-filter.tsx
index c1f9dadee0..6832729acc 100644
--- a/packages/admin/dashboard/src/components/table/data-table/data-table-filter/string-filter.tsx
+++ b/packages/admin/dashboard/src/components/table/data-table/data-table-filter/string-filter.tsx
@@ -1,12 +1,11 @@
-import { XMarkMini } from "@medusajs/icons"
-import { Input, Label, Text, clx } from "@medusajs/ui"
+import { Input, Label, clx } from "@medusajs/ui"
 import * as Popover from "@radix-ui/react-popover"
 import { debounce } from "lodash"
 import { ChangeEvent, useCallback, useEffect, useState } from "react"
-import { useTranslation } from "react-i18next"
 import { useSelectedParams } from "../hooks"
 import { useDataTableFilterContext } from "./context"
 import { IFilter } from "./types"
+import FilterChip from "./filter-chip"
 
 type StringFilterProps = IFilter
 
@@ -25,6 +24,8 @@ export const StringFilter = ({
 
   const query = selectedParams.get()
 
+  const [previousValue, setPreviousValue] = useState<string | undefined>(query?.[0])
+
   // eslint-disable-next-line react-hooks/exhaustive-deps
   const debouncedOnChange = useCallback(
     debounce((e: ChangeEvent<HTMLInputElement>) => {
@@ -49,6 +50,7 @@ export const StringFilter = ({
 
   const handleOpenChange = (open: boolean) => {
     setOpen(open)
+    setPreviousValue(query?.[0])
 
     if (timeoutId) {
       clearTimeout(timeoutId)
@@ -68,7 +70,9 @@ export const StringFilter = ({
 
   return (
     <Popover.Root modal open={open} onOpenChange={handleOpenChange}>
-      <StringDisplay
+      <FilterChip
+        hasOperator
+        hadPreviousValue={!!previousValue}
         label={label}
         value={query?.[0]}
         onRemove={handleRemove}
@@ -117,84 +121,3 @@ export const StringFilter = ({
     </Popover.Root>
   )
 }
-
-const StringDisplay = ({
-  label,
-  value,
-  readonly,
-  onRemove,
-}: {
-  label: string
-  value?: string
-  readonly?: boolean
-  onRemove: () => void
-}) => {
-  const { t } = useTranslation()
-
-  return (
-    <Popover.Trigger asChild>
-      <div
-        className={clx(
-          "bg-ui-bg-field transition-fg shadow-borders-base text-ui-fg-subtle flex cursor-pointer select-none items-center overflow-hidden rounded-md",
-          {
-            "hover:bg-ui-bg-field-hover": !readonly,
-            "data-[state=open]:bg-ui-bg-field-hover": !readonly,
-          }
-        )}
-      >
-        <div
-          className={clx(
-            "flex items-center justify-center whitespace-nowrap px-2 py-1",
-            {
-              "border-r": !!value,
-            }
-          )}
-        >
-          <Text size="small" weight="plus" leading="compact">
-            {label}
-          </Text>
-        </div>
-        <div className="flex w-full items-center overflow-hidden">
-          {!!value && (
-            <div className="border-r p-1 px-2">
-              <Text
-                size="small"
-                weight="plus"
-                leading="compact"
-                className="text-ui-fg-muted"
-              >
-                {t("general.is")}
-              </Text>
-            </div>
-          )}
-          {!!value && (
-            <div className="flex-1 overflow-hidden border-r p-1 px-2">
-              <Text
-                size="small"
-                leading="compact"
-                weight="plus"
-                className="truncate text-nowrap"
-              >
-                {value}
-              </Text>
-            </div>
-          )}
-        </div>
-        {!readonly && !!value && (
-          <div>
-            <button
-              onClick={onRemove}
-              className={clx(
-                "text-ui-fg-muted transition-fg flex items-center justify-center p-1",
-                "hover:bg-ui-bg-subtle-hover",
-                "active:bg-ui-bg-subtle-pressed active:text-ui-fg-base"
-              )}
-            >
-              <XMarkMini />
-            </button>
-          </div>
-        )}
-      </div>
-    </Popover.Trigger>
-  )
-}
diff --git a/packages/admin/dashboard/src/routes/locations/location-service-zone-manage-areas/location-service-zone-manage-areas.tsx b/packages/admin/dashboard/src/routes/locations/location-service-zone-manage-areas/location-service-zone-manage-areas.tsx
index 7a1127fa1b..922454b15f 100644
--- a/packages/admin/dashboard/src/routes/locations/location-service-zone-manage-areas/location-service-zone-manage-areas.tsx
+++ b/packages/admin/dashboard/src/routes/locations/location-service-zone-manage-areas/location-service-zone-manage-areas.tsx
@@ -9,7 +9,7 @@ export const LocationServiceZoneManageAreas = () => {
 
   const { stock_location, isPending, isFetching, isError, error } =
     useStockLocation(location_id!, {
-      fields: "*fulfillment_sets.service_zones.geo_zones",
+      fields: "*fulfillment_sets.service_zones.geo_zones,fulfillment_sets.service_zones.name",
     })
 
   const zone = stock_location?.fulfillment_sets