Fix default filter behaviour (#820)

Co-authored-by: philon- <[email protected]>

packages/web/src/components/generic/Filter/FilterControl.tsx

@@ -16,20 +16,19 @@ import {
 import { cn } from "@core/utils/cn.ts";
 import { debounce } from "@core/utils/debounce.ts";
 import { Protobuf } from "@meshtastic/core";
-import type { TFunction } from "i18next";
 import { FunnelIcon } from "lucide-react";
 import {
   type ComponentProps,
   type ReactNode,
   useCallback,
   useEffect,
+  useMemo,
   useRef,
   useState,
 } from "react";
 import { useTranslation } from "react-i18next";
 
 const DEBOUNCE_DELAY_MS = 250;
-const BATTERY_STATUS_PLUGGED_IN_VALUE = 101;
 
 type PopoverContentProps = ComponentProps<typeof PopoverContent>;
 
@@ -47,88 +46,57 @@ interface FilterControlProps {
   children?: ReactNode;
 }
 
-interface HopsLabelProps {
+interface RangeLabelContentProps {
-  hopsAway: number[];
+  range: [number, number];
-  t: TFunction<"ui", undefined>;
+  defaultRange: [number, number];
+  format?: (ts: number) => ReactNode;
+  initialLabel?: ReactNode;
+  customLabel?: { start?: string; end?: string };
 }
-function HopsLabelContent({ hopsAway, t }: HopsLabelProps) {
-  const startHops = hopsAway[0];
-  const endHops = hopsAway[1];
 
-  return (
+function RangeLabelContent({
-    <>
+  range,
-      {t("hops.text", {
+  defaultRange,
-        value: startHops === 0 ? t("hops.direct") : startHops,
+  format,
-      })}
+  initialLabel,
-      {startHops !== endHops ? ` — ${endHops}` : ""}
+  customLabel,
-    </>
+}: RangeLabelContentProps) {
-  );
+  const [start, end] = range;
-}
+  const [min, max] = defaultRange;
-
+  const unequal = start !== end;
-interface LastHeardLabelProps {
-  lastHeardRange: number[];
-  defaultMaxLastHeard: number;
-  formatTS: (ts: number) => ReactNode;
-  t: TFunction<"ui", undefined>;
-}
-function LastHeardLabelContent({
-  lastHeardRange,
-  defaultMaxLastHeard,
-  formatTS,
-  t,
-}: LastHeardLabelProps) {
-  const [start, end] = lastHeardRange;
-  return (
-    <>
-      {t("lastHeard.labelText", { value: "" })}
-      <br />
-      {start === 0 ? (
-        t("lastHeard.nowLabel")
-      ) : (
-        <>
-          {start === defaultMaxLastHeard && ">"}
-          {formatTS(start)}
-        </>
-      )}
-      {start !== end && (
-        <>
-          {" — "}
-          {end === defaultMaxLastHeard && ">"}
-          {formatTS(end)}
-        </>
-      )}
-    </>
-  );
-}
 
-interface BatteryLevelLabelProps {
+  const fmtStart = format ? format(start) : start;
-  batteryLevelRange: (number | undefined)[];
+  const fmtEnd = format ? format(end) : end;
-  t: TFunction<"ui", undefined>;
-}
-function BatteryLevelLabelContent({
-  batteryLevelRange,
-  t,
-}: BatteryLevelLabelProps) {
-  const [start, end] = batteryLevelRange;
-
-  const formatBatteryValue = (value: number | undefined) => {
-    if (value === undefined) {
-      return "";
-    }
-    return value === BATTERY_STATUS_PLUGGED_IN_VALUE
-      ? t("batteryStatus.pluggedIn")
-      : `${value}%`;
-  };
 
   return (
     <>
-      {t("batteryLevel.labelText", {
+      {initialLabel}
-        value: formatBatteryValue(start),
+      {start === min
-      })}
+        ? (customLabel?.start ?? (
-      {start !== end && typeof end !== "undefined" && (
+            <>
+              {"<"}
+              {fmtStart}
+            </>
+          ))
+        : start === max
+          ? (customLabel?.end ?? (
+              <>
+                {">"}
+                {fmtEnd}
+              </>
+            ))
+          : fmtStart}
+      {unequal && (
         <>
-          {" – "}
+          {" — "}
-          {formatBatteryValue(end)}
+          {end === max
+            ? (customLabel?.end ?? (
+                <>
+                  {">"}
+                  {fmtEnd}
+                </>
+              ))
+            : fmtEnd}
         </>
       )}
     </>
@@ -190,18 +158,20 @@ export function FilterControl({
   );
 
   const handleBoolChange = useCallback(
-    <K extends keyof FilterState>(key: K, value: string | boolean) => {
+    <K extends keyof FilterState>(
-      const typedValue =
+      key: K,
-        value === true || value === "true"
+      value: string | boolean | undefined,
-          ? true
+    ) => {
-          : value === false || value === "false"
+      let typedValue: boolean | undefined;
-            ? false
+      if (value === true || value === "true") {
-            : undefined;
+        typedValue = true;
+      } else if (value === false || value === "false") {
+        typedValue = false;
+      } else {
+        typedValue = undefined;
+      }
 
-      setFilterState((prev) => ({
+      setFilterState((prev) => ({ ...prev, [key]: typedValue }));
-        ...prev,
-        [key]: typedValue,
-      }));
     },
     [setFilterState],
   );
@@ -220,6 +190,21 @@ export function FilterControl({
     [],
   );
 
+  const roleOptions = useMemo(
+    () =>
+      Object.values(Protobuf.Config.Config_DeviceConfig_Role).filter(
+        (v): v is number => typeof v === "number",
+      ),
+    [],
+  );
+  const hwModelOptions = useMemo(
+    () =>
+      Object.values(Protobuf.Mesh.HardwareModel).filter(
+        (v): v is number => typeof v === "number",
+      ),
+    [],
+  );
+
   return (
     <Popover>
       <PopoverTrigger asChild>
@@ -229,9 +214,8 @@ export function FilterControl({
             "rounded",
             "text-slate-600 hover:text-slate-700 bg-slate-100 hover:bg-slate-200 active:bg-slate-300",
             "dark:text-slate-400 hover:dark:text-slate-400 dark:bg-slate-700 hover:dark:bg-slate-800 dark:active:bg-slate-950",
-            isDirty
+            isDirty &&
-              ? "text-slate-100 dark:text-slate-300 bg-green-600 dark:bg-green-700 hover:bg-green-700 dark:hover:bg-green-800 hover:text-slate-200 dark:hover:text-slate-300 active:bg-green-800 dark:active:bg-green-900"
+              "text-slate-100 dark:text-slate-300 bg-green-600 dark:bg-green-700 hover:bg-green-700 dark:hover:bg-green-800 hover:text-slate-200 dark:hover:text-slate-300 active:bg-green-800 dark:active:bg-green-900",
-              : "",
             parameters?.popoverTriggerClassName,
           )}
           aria-label={t("filter.label")}
@@ -269,30 +253,31 @@ export function FilterControl({
                 </div>
               )}
               <FilterSlider
-                label={t("hops.label")}
                 filterKey="hopsAway"
                 filterState={localFilterState}
                 defaultFilterValues={defaultFilterValues}
                 onChange={handleRangeChange}
                 labelContent={
-                  <HopsLabelContent
+                  <RangeLabelContent
-                    hopsAway={localFilterState.hopsAway}
+                    range={localFilterState.hopsAway}
-                    t={t}
+                    defaultRange={defaultFilterValues.hopsAway}
+                    initialLabel={`${t("hops.label")}: `}
+                    customLabel={{ start: "0", end: "7" }}
                   />
                 }
               />
               <FilterSlider
-                label={t("lastHeard.label")}
                 filterKey="lastHeard"
                 filterState={localFilterState}
                 defaultFilterValues={defaultFilterValues}
                 onChange={handleRangeChange}
                 labelContent={
-                  <LastHeardLabelContent
+                  <RangeLabelContent
-                    lastHeardRange={localFilterState.lastHeard}
+                    range={localFilterState.lastHeard}
-                    defaultMaxLastHeard={defaultFilterValues.lastHeard[1]}
+                    defaultRange={defaultFilterValues.lastHeard}
-                    formatTS={formatTS}
+                    format={formatTS}
-                    t={t}
+                    initialLabel={`${t("lastHeard.label")}: `}
+                    customLabel={{ start: t("lastHeard.nowLabel") }}
                   />
                 }
               />
@@ -314,11 +299,17 @@ export function FilterControl({
 
             <FilterAccordionItem label={t("metrics.label")}>
               <FilterSlider
-                label={t("snr.label")}
                 filterKey="snr"
                 filterState={localFilterState}
                 defaultFilterValues={defaultFilterValues}
                 onChange={handleRangeChange}
+                labelContent={
+                  <RangeLabelContent
+                    range={localFilterState.snr}
+                    defaultRange={defaultFilterValues.snr}
+                    initialLabel={`${t("snr.label")}: `}
+                  />
+                }
               />
               <FilterSlider
                 label={t("channelUtilization.label")}
@@ -335,24 +326,36 @@ export function FilterControl({
                 onChange={handleRangeChange}
               />
               <FilterSlider
-                label={t("batteryLevel.label")}
                 filterKey="batteryLevel"
                 filterState={localFilterState}
                 defaultFilterValues={defaultFilterValues}
                 onChange={handleRangeChange}
                 labelContent={
-                  <BatteryLevelLabelContent
+                  <RangeLabelContent
-                    batteryLevelRange={localFilterState.batteryLevel}
+                    range={localFilterState.batteryLevel}
-                    t={t}
+                    defaultRange={defaultFilterValues.batteryLevel}
+                    initialLabel={`${t("batteryLevel.label")}: `}
+                    customLabel={{
+                      start: "0",
+                      end: t("batteryStatus.pluggedIn"),
+                    }}
                   />
                 }
               />
               <FilterSlider
-                label={t("batteryVoltage.label")}
                 filterKey="voltage"
                 filterState={localFilterState}
                 defaultFilterValues={defaultFilterValues}
                 onChange={handleRangeChange}
+                step={0.1}
+                labelContent={
+                  <RangeLabelContent
+                    range={localFilterState.voltage}
+                    defaultRange={defaultFilterValues.voltage}
+                    initialLabel={`${t("batteryVoltage.label")}: `}
+                    customLabel={{ start: "0" }}
+                  />
+                }
               />
             </FilterAccordionItem>
 
@@ -361,11 +364,11 @@ export function FilterControl({
                 filterKey="role"
                 filterState={filterState}
                 setFilterState={setFilterState}
-                options={Object.values(
+                options={roleOptions}
-                  Protobuf.Config.Config_DeviceConfig_Role,
-                ).filter((v): v is number => typeof v === "number")}
                 getLabel={(val) =>
-                  formatEnumLabel(Protobuf.Config.Config_DeviceConfig_Role[val])
+                  formatEnumLabel(
+                    Protobuf.Config.Config_DeviceConfig_Role[val] ?? "UNSET",
+                  )
                 }
               />
             </FilterAccordionItem>
@@ -375,11 +378,9 @@ export function FilterControl({
                 filterKey="hwModel"
                 filterState={filterState}
                 setFilterState={setFilterState}
-                options={Object.values(Protobuf.Mesh.HardwareModel).filter(
+                options={hwModelOptions}
-                  (v): v is number => typeof v === "number",
-                )}
                 getLabel={(val) =>
-                  formatEnumLabel(Protobuf.Mesh.HardwareModel[val])
+                  formatEnumLabel(Protobuf.Mesh.HardwareModel[val] ?? "UNKNOWN")
                 }
               />
             </FilterAccordionItem>

packages/web/src/components/generic/Filter/useFilterNode.test.ts

@@ -180,4 +180,56 @@ describe("useFilterNode", () => {
       expect(isFilterDirty(modified)).toBe(true);
     });
   });
+
+  describe("default-boundary semantics", () => {
+    const node = createMockNode();
+
+    it("lastHeard: end at default max means 'any age'", () => {
+      const {
+        lastHeard: [lastHeardMin, lastHeardMax],
+      } = defaultFilterValues;
+      const veryOld = { ...node, lastHeard: lastHeardMax + 1 }; // older than slider max
+      expect(
+        nodeFilter(veryOld, { lastHeard: [lastHeardMin, lastHeardMax] }),
+      ).toBe(true); // open upper
+      expect(
+        nodeFilter(veryOld, { lastHeard: [lastHeardMin, lastHeardMax - 1] }),
+      ).toBe(false); // now bounded
+    });
+
+    it("snr: max at default means no upper bound", () => {
+      const {
+        snr: [snrMin, snrMax],
+      } = defaultFilterValues;
+      const hiSnr = { ...node, snr: snrMax + 1 }; // above slider max
+      expect(nodeFilter(hiSnr, { snr: [snrMin, snrMax] })).toBe(true); // open upper
+      expect(nodeFilter(hiSnr, { snr: [snrMin, snrMax - 1] })).toBe(false); // bounded
+    });
+
+    it("snr: min at default means no lower bound", () => {
+      const {
+        snr: [snrMin, snrMax],
+      } = defaultFilterValues;
+      const loSnr = { ...node, snr: snrMin - 1 }; // below slider min
+      expect(nodeFilter(loSnr, { snr: [snrMin, snrMax] })).toBe(true); // open lower
+      expect(nodeFilter(loSnr, { snr: [snrMin + 1, snrMax] })).toBe(false); // bounded
+    });
+
+    it("voltage: max at default means no upper bound", () => {
+      const {
+        voltage: [voltageMin, voltageMax],
+      } = defaultFilterValues;
+      const hiV = {
+        ...node,
+        deviceMetrics: {
+          ...node.deviceMetrics!,
+          voltage: voltageMax + 1,
+        },
+      } satisfies Protobuf.Mesh.NodeInfo;
+      expect(nodeFilter(hiV, { voltage: [voltageMin, voltageMax] })).toBe(true); // open upper
+      expect(
+        nodeFilter(hiV, { voltage: [voltageMin, voltageMax - 0.01] }),
+      ).toBe(false); // bounded
+    });
+  });
 });

packages/web/src/components/generic/Filter/useFilterNode.ts

@@ -127,7 +127,12 @@ export function useFilterNode() {
       }
 
       const snr = node.snr ?? -20;
-      if (snr < filterState.snr[0] || snr > filterState.snr[1]) {
+      if (
+        (snr < filterState.snr[0] &&
+          filterState.snr[0] !== defaultFilterValues.snr[0]) ||
+        (snr > filterState.snr[1] &&
+          filterState.snr[1] !== defaultFilterValues.snr[1])
+      ) {
         return false;
       }
 
@@ -158,7 +163,8 @@ export function useFilterNode() {
       const voltage = node.deviceMetrics?.voltage ?? 0;
       if (
         voltage < filterState.voltage[0] ||
-        voltage > filterState.voltage[1]
+        (voltage > filterState.voltage[1] &&
+          filterState.voltage[1] !== defaultFilterValues.voltage[1])
       ) {
         return false;
       }