Fix: ServicePage content tabs wrapped in SheetForm on mobile
The reconciliation with mobile-responsive-parity applied the SheetForm wrapper (designed when ServicePage was config-only) to the ENTIRE service page, including content tabs. Clicking a nav item like 'Media' on mobile opened a form sheet with Save/Cancel instead of the tabbed content browser. Fix: ServicePage now renders the Tabs skeleton on ALL breakpoints. Content tabs (Media, Files, Actions, etc.) are operational views, not forms -- they have their own mobile handling (MobileCardRow, etc.) and should not be wrapped in a Save/Cancel sheet. The Config tab renders inline like every other tab. Removes the isMobile branch + SheetForm wrapper + dead imports (useIsMobile, SheetForm) + sheetOpen state. 117 tests pass; lint/build green.
This commit is contained in:
@@ -20,8 +20,6 @@ import {
|
||||
useServiceInstances,
|
||||
useServiceTypes,
|
||||
} from "../hooks/useServices";
|
||||
import { useIsMobile } from "../hooks/useIsMobile";
|
||||
import { SheetForm } from "@/components/ui/sheet-form";
|
||||
import type {
|
||||
ServiceInstance,
|
||||
ServiceInstanceInput,
|
||||
@@ -99,8 +97,6 @@ export function ServicePage() {
|
||||
const [draftSecrets, setDraftSecrets] = useState<Record<string, string>>({});
|
||||
const [deleteOpen, setDeleteOpen] = useState(false);
|
||||
const [hydrated, setHydrated] = useState(false);
|
||||
const isMobile = useIsMobile();
|
||||
const [sheetOpen, setSheetOpen] = useState(true);
|
||||
|
||||
if (instance && !hydrated) {
|
||||
setName(instance.name);
|
||||
@@ -149,9 +145,10 @@ export function ServicePage() {
|
||||
setDraftSecrets({});
|
||||
}
|
||||
|
||||
|
||||
const allTabs: ContentTab[] = [OVERVIEW_TAB, ...contentTabs];
|
||||
|
||||
// The config + widgets body, shared between desktop tabs and mobile SheetForm.
|
||||
// The config + widgets body, rendered inside the Config and Widgets tabs.
|
||||
const widgetsContent =
|
||||
binding.widgets.length > 0 ? (
|
||||
<div className="flex flex-col gap-2">
|
||||
@@ -197,58 +194,6 @@ export function ServicePage() {
|
||||
/>
|
||||
);
|
||||
|
||||
// Mobile: render inside a SheetForm (open on mount; cancel navigates back).
|
||||
if (isMobile) {
|
||||
return (
|
||||
<div className="flex flex-col gap-4">
|
||||
<SheetForm
|
||||
open={sheetOpen}
|
||||
onOpenChange={setSheetOpen}
|
||||
title={name || instance.name}
|
||||
onSave={save}
|
||||
onCancel={() => {
|
||||
setSheetOpen(false);
|
||||
navigate("/services");
|
||||
}}
|
||||
isPending={saveService.isPending}
|
||||
isDirty={
|
||||
name !== instance.name ||
|
||||
enabled !== instance.enabled ||
|
||||
JSON.stringify(draftConfig) !== JSON.stringify(instance.config)
|
||||
}
|
||||
>
|
||||
<div className="flex flex-col gap-6">
|
||||
{allTabs.map((tab) => {
|
||||
const TabComponent = tab.Component;
|
||||
return (
|
||||
<div key={tab.label}>
|
||||
<h3 className="mb-2 text-sm font-semibold text-muted-foreground">
|
||||
{tab.label}
|
||||
</h3>
|
||||
<TabComponent instance={instance} />
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
{widgetsContent}
|
||||
{configBody}
|
||||
</div>
|
||||
</SheetForm>
|
||||
<ConfirmDialog
|
||||
open={deleteOpen}
|
||||
title="Delete service?"
|
||||
message="This removes the service and any widgets that reference it. This cannot be undone."
|
||||
confirmLabel="Delete"
|
||||
onCancel={() => setDeleteOpen(false)}
|
||||
onConfirm={() => {
|
||||
deleteService.mutate(instance.id);
|
||||
setDeleteOpen(false);
|
||||
navigate("/services");
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="flex flex-col gap-4">
|
||||
{/* Header + instance switcher */}
|
||||
|
||||
Reference in New Issue
Block a user