From 6a2e7d06109eb65b42bbe4b851bc2b5fc8b687a7 Mon Sep 17 00:00:00 2001 From: ajnart Date: Tue, 28 Jul 2026 15:25:44 +0200 Subject: [PATCH] fix: address mobile dashboard review findings --- .../board/mobile/mobile-board.module.css | 2 +- packages/api/src/router/widgets/beszel.ts | 2 ++ packages/api/src/settle-integrations.spec.ts | 31 +++++++++++++++++++ .../src/beszel/_shared/mobile-summary.tsx | 13 ++++++-- .../src/beszel/_shared/query-status.spec.ts | 11 +++++++ .../src/beszel/_shared/query-status.ts | 2 ++ .../widgets/src/beszel/_shared/stats-view.tsx | 21 ++++--------- .../widgets/src/filter-storage-volumes.ts | 2 +- .../src/system-disks/mobile-summary.spec.ts | 15 +++++++++ .../src/system-disks/mobile-summary.ts | 2 ++ .../widgets/src/weather/temperature.spec.ts | 20 ++++++++++++ packages/widgets/src/weather/temperature.ts | 6 ++++ 12 files changed, 107 insertions(+), 20 deletions(-) create mode 100644 packages/widgets/src/beszel/_shared/query-status.spec.ts create mode 100644 packages/widgets/src/beszel/_shared/query-status.ts create mode 100644 packages/widgets/src/system-disks/mobile-summary.spec.ts create mode 100644 packages/widgets/src/system-disks/mobile-summary.ts create mode 100644 packages/widgets/src/weather/temperature.spec.ts create mode 100644 packages/widgets/src/weather/temperature.ts diff --git a/apps/nextjs/src/components/board/mobile/mobile-board.module.css b/apps/nextjs/src/components/board/mobile/mobile-board.module.css index 78026b2f1..80783dd08 100644 --- a/apps/nextjs/src/components/board/mobile/mobile-board.module.css +++ b/apps/nextjs/src/components/board/mobile/mobile-board.module.css @@ -13,7 +13,7 @@ padding: 0; margin: -1px; overflow: hidden; - clip: rect(0, 0, 0, 0); + clip-path: inset(50%); white-space: nowrap; border: 0; } diff --git a/packages/api/src/router/widgets/beszel.ts b/packages/api/src/router/widgets/beszel.ts index c52ca1717..65fdf53d1 100644 --- a/packages/api/src/router/widgets/beszel.ts +++ b/packages/api/src/router/widgets/beszel.ts @@ -44,6 +44,7 @@ export const beszelRouter = createTRPCRouter({ }; }, { + throwOnAllFailure: true, fallback: (integration, error) => ({ integrationId: integration.id, integrationName: integration.name, @@ -116,6 +117,7 @@ export const beszelRouter = createTRPCRouter({ }; }, { + throwOnAllFailure: true, fallback: (integration, error) => ({ integrationId: integration.id, integrationName: integration.name, diff --git a/packages/api/src/settle-integrations.spec.ts b/packages/api/src/settle-integrations.spec.ts index 52f0d3697..ca032b700 100644 --- a/packages/api/src/settle-integrations.spec.ts +++ b/packages/api/src/settle-integrations.spec.ts @@ -23,6 +23,22 @@ describe("settleIntegrationQueries", () => { expect(results).toEqual(["Second"]); }); + it("retains fallback and successful results for a partial failure", async () => { + const results = await settleIntegrationQueries( + integrations, + vi.fn(async (integration) => { + if (integration.id === "first") throw new Error("offline"); + return integration.name; + }), + { + throwOnAllFailure: true, + fallback: (integration) => `${integration.name} unavailable`, + }, + ); + + expect(results).toEqual(["First unavailable", "Second"]); + }); + it("throws when every integration fails and throwOnAllFailure is enabled", async () => { await expect( settleIntegrationQueries( @@ -35,6 +51,21 @@ describe("settleIntegrationQueries", () => { ).rejects.toThrow("offline"); }); + it("throws on total failure even when fallbacks are configured", async () => { + await expect( + settleIntegrationQueries( + integrations, + vi.fn(async () => { + throw new Error("offline"); + }), + { + throwOnAllFailure: true, + fallback: (integration) => `${integration.name} unavailable`, + }, + ), + ).rejects.toThrow("offline"); + }); + it("does not treat an empty integration list as a failure", async () => { await expect( settleIntegrationQueries( diff --git a/packages/widgets/src/beszel/_shared/mobile-summary.tsx b/packages/widgets/src/beszel/_shared/mobile-summary.tsx index 7f1caa6a0..d573b50e7 100644 --- a/packages/widgets/src/beszel/_shared/mobile-summary.tsx +++ b/packages/widgets/src/beszel/_shared/mobile-summary.tsx @@ -5,6 +5,7 @@ import { useI18n } from "@homarr/translation/client"; import { WidgetMobileLoading, WidgetMobileSummary } from "../../common/mobile-summary"; import { useBeszelFilteredSystems } from "./hooks"; +import { hasStaleIntegrationData } from "./query-status"; interface BeszelSystemsMobileSummaryProps { integrationIds: string[]; @@ -18,12 +19,18 @@ export const BeszelSystemsMobileSummary = ({ statusFilter, }: BeszelSystemsMobileSummaryProps) => { const t = useI18n(); - const { data: results = [], error, isPending } = clientApi.widget.beszel.getSystems.useQuery({ integrationIds }); + const { + data: results = [], + error, + isPending, + isLoadingError, + isRefetchError, + } = clientApi.widget.beszel.getSystems.useQuery({ integrationIds }); const systems = useBeszelFilteredSystems(results, statusFilter); const failedResults = results.filter((result) => "error" in result); if (isPending) return ; - if (error) throw error; + if (isLoadingError) throw error; if (results.length > 0 && failedResults.length === results.length) { throw new Error(String(failedResults[0]?.error ?? "Unable to connect to Beszel")); } @@ -38,7 +45,7 @@ export const BeszelSystemsMobileSummary = ({ up, attention: systems.length - up, })} - isStale={failedResults.length > 0} + isStale={hasStaleIntegrationData(isRefetchError, failedResults)} /> ); }; diff --git a/packages/widgets/src/beszel/_shared/query-status.spec.ts b/packages/widgets/src/beszel/_shared/query-status.spec.ts new file mode 100644 index 000000000..7663babb7 --- /dev/null +++ b/packages/widgets/src/beszel/_shared/query-status.spec.ts @@ -0,0 +1,11 @@ +import { describe, expect, test } from "vitest"; + +import { hasStaleIntegrationData } from "./query-status"; + +describe("Beszel query status", () => { + test("marks cached data stale after outer or per-integration failures", () => { + expect(hasStaleIntegrationData(true, [])).toBe(true); + expect(hasStaleIntegrationData(false, [{ error: "offline" }])).toBe(true); + expect(hasStaleIntegrationData(false, [])).toBe(false); + }); +}); diff --git a/packages/widgets/src/beszel/_shared/query-status.ts b/packages/widgets/src/beszel/_shared/query-status.ts new file mode 100644 index 000000000..4d4827486 --- /dev/null +++ b/packages/widgets/src/beszel/_shared/query-status.ts @@ -0,0 +1,2 @@ +export const hasStaleIntegrationData = (isRefetchError: boolean, failedResults: readonly unknown[]) => + isRefetchError || failedResults.length > 0; diff --git a/packages/widgets/src/beszel/_shared/stats-view.tsx b/packages/widgets/src/beszel/_shared/stats-view.tsx index c2042a4a9..57ff4b40b 100644 --- a/packages/widgets/src/beszel/_shared/stats-view.tsx +++ b/packages/widgets/src/beszel/_shared/stats-view.tsx @@ -20,22 +20,17 @@ import { useSystemChartData, } from "./chart"; import { chartAxisFormatters, formatByteRate, formatGB, formatPercent, formatStorageBytes } from "./format"; -import type { BeszelTooltipLayer } from "./tooltip"; import { makeTooltipProps } from "./tooltip"; import { useLiveStats } from "./use-live-stats"; const CHART_HEIGHT = 180; const MB = 1024 * 1024; -const makeTooltipSet = (layer: BeszelTooltipLayer) => ({ - percent: makeTooltipProps(formatPercent, false, layer), - gigabytes: makeTooltipProps(formatGB, true, layer), - rate: makeTooltipProps(formatByteRate, true, layer), - percentTotal: makeTooltipProps(formatPercent, true, layer), - bytesTotal: makeTooltipProps(formatStorageBytes, true, layer), -}); -const tooltipSets = { - background: makeTooltipSet("background"), - modal: makeTooltipSet("modal"), +const tooltips = { + percent: makeTooltipProps(formatPercent), + gigabytes: makeTooltipProps(formatGB, true), + rate: makeTooltipProps(formatByteRate, true), + percentTotal: makeTooltipProps(formatPercent, true), + bytesTotal: makeTooltipProps(formatStorageBytes, true), }; const ChartSkeleton = () => ( @@ -66,7 +61,6 @@ interface BeszelStatsViewProps { visibility: BeszelStatsVisibility; columns: 1 | 2; onSwitchToHistorical?: () => void; - tooltipLayer?: BeszelTooltipLayer; } const whenVisible = (visible: boolean, data: T | undefined) => (visible ? data : undefined); @@ -78,14 +72,11 @@ export function BeszelStatsView({ visibility, columns, onSwitchToHistorical, - tooltipLayer = "background", }: BeszelStatsViewProps) { const t = useScopedI18n("widget.beszelSystemStats"); const locale = useCurrentIntlLocale(); const showDocker = visibility.dockerCpu || visibility.dockerMemory || visibility.dockerNetwork; const isLive = timePeriod === "1m"; - const tooltips = tooltipSets[tooltipLayer]; - const historicalQuery = clientApi.widget.beszel.getSystemStats.useQuery( { integrationIds, systemId, timePeriod, includeDocker: showDocker }, { refetchInterval: isLive ? false : 5_000, enabled: !isLive && systemId !== "" }, diff --git a/packages/widgets/src/filter-storage-volumes.ts b/packages/widgets/src/filter-storage-volumes.ts index 9c406c65e..f5d294508 100644 --- a/packages/widgets/src/filter-storage-volumes.ts +++ b/packages/widgets/src/filter-storage-volumes.ts @@ -28,7 +28,7 @@ export const toScopedStorageVolumeValue = (integrationId: string, value: string) return `${integrationId}:${volumeName}`; }; -const storageDeviceNamesMatch = (leftDeviceName: string, rightDeviceName: string): boolean => { +export const storageDeviceNamesMatch = (leftDeviceName: string, rightDeviceName: string): boolean => { return ( leftDeviceName === rightDeviceName || normalizeStorageDeviceName(leftDeviceName) === normalizeStorageDeviceName(rightDeviceName) diff --git a/packages/widgets/src/system-disks/mobile-summary.spec.ts b/packages/widgets/src/system-disks/mobile-summary.spec.ts new file mode 100644 index 000000000..ebe754c57 --- /dev/null +++ b/packages/widgets/src/system-disks/mobile-summary.spec.ts @@ -0,0 +1,15 @@ +import { describe, expect, test } from "vitest"; + +import { formatDiskTemperature } from "./mobile-summary"; + +describe("formatDiskTemperature", () => { + test("preserves a valid zero-degree reading", () => { + expect(formatDiskTemperature(0, true)).toBe("0°C"); + }); + + test("omits unavailable or disabled readings", () => { + expect(formatDiskTemperature(null, true)).toBeUndefined(); + expect(formatDiskTemperature(undefined, true)).toBeUndefined(); + expect(formatDiskTemperature(20, false)).toBeUndefined(); + }); +}); diff --git a/packages/widgets/src/system-disks/mobile-summary.ts b/packages/widgets/src/system-disks/mobile-summary.ts new file mode 100644 index 000000000..123d0dd7d --- /dev/null +++ b/packages/widgets/src/system-disks/mobile-summary.ts @@ -0,0 +1,2 @@ +export const formatDiskTemperature = (temperature: number | null | undefined, shouldShow: boolean) => + shouldShow && temperature != null ? `${temperature}°C` : undefined; diff --git a/packages/widgets/src/weather/temperature.spec.ts b/packages/widgets/src/weather/temperature.spec.ts new file mode 100644 index 000000000..d22b96619 --- /dev/null +++ b/packages/widgets/src/weather/temperature.spec.ts @@ -0,0 +1,20 @@ +import { describe, expect, test } from "vitest"; + +import { getPreferredUnit } from "./temperature"; + +describe("getPreferredUnit", () => { + test("preserves zero in Celsius and Fahrenheit", () => { + expect(getPreferredUnit(0)).toBe("0.0°C"); + expect(getPreferredUnit(0, true)).toBe("32.0°F"); + }); + + test("only uses the unavailable placeholder for missing values", () => { + expect(getPreferredUnit(undefined)).toBe("?"); + }); + + test("respects the decimal preference", () => { + expect(getPreferredUnit(0, false, true)).toBe("0°C"); + expect(getPreferredUnit(0, true, true)).toBe("32°F"); + expect(getPreferredUnit(12.6, false, true)).toBe("13°C"); + }); +}); diff --git a/packages/widgets/src/weather/temperature.ts b/packages/widgets/src/weather/temperature.ts new file mode 100644 index 000000000..73edf7f87 --- /dev/null +++ b/packages/widgets/src/weather/temperature.ts @@ -0,0 +1,6 @@ +export const getPreferredUnit = (value?: number, isFahrenheit = false, disableTemperatureDecimals = false): string => + value !== undefined + ? isFahrenheit + ? `${(value * (9 / 5) + 32).toFixed(disableTemperatureDecimals ? 0 : 1)}°F` + : `${value.toFixed(disableTemperatureDecimals ? 0 : 1)}°C` + : "?";