Skip to content

Commit ec54757

Browse files
feat: Add confirm dialog when closing tile editor w/ changes (#1746)
Fixes: HDX-3201 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 3c1511d commit ec54757

8 files changed

Lines changed: 195 additions & 75 deletions

File tree

‎.changeset/cuddly-cycles-speak.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hyperdx/app": patch
3+
---
4+
5+
feat: Add confirm dialog when closing tile editor w/ changes

‎packages/app/pages/_app.tsx‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import { ibmPlexMono, inter, roboto, robotoMono } from '@/fonts';
2727
import { AppThemeProvider, useAppTheme } from '@/theme/ThemeProvider';
2828
import { ThemeWrapper } from '@/ThemeWrapper';
2929
import { NextApiConfigResponseData } from '@/types';
30-
import { useConfirmModal } from '@/useConfirm';
30+
import { ConfirmProvider } from '@/useConfirm';
3131
import { QueryParamProvider as HDXQueryParamProvider } from '@/useQueryParam';
3232
import { useUserPreferences } from '@/useUserPreferences';
3333

@@ -90,11 +90,9 @@ function AppHeadContent() {
9090
function AppContent({
9191
Component,
9292
pageProps,
93-
confirmModal,
9493
}: {
9594
Component: NextPageWithLayout;
9695
pageProps: AppProps['pageProps'];
97-
confirmModal: React.ReactNode;
9896
}) {
9997
const { userPreferences } = useUserPreferences();
10098
const { themeName } = useAppTheme();
@@ -122,15 +120,14 @@ function AppContent({
122120
fontFamily={selectedMantineFont}
123121
colorScheme={userPreferences.colorMode === 'dark' ? 'dark' : 'light'}
124122
>
125-
{getLayout(<Component {...pageProps} />)}
126-
{confirmModal}
123+
<ConfirmProvider>
124+
{getLayout(<Component {...pageProps} />)}
125+
</ConfirmProvider>
127126
</ThemeWrapper>
128127
);
129128
}
130129

131130
export default function MyApp({ Component, pageProps }: AppPropsWithLayout) {
132-
const confirmModal = useConfirmModal();
133-
134131
// port to react query ? (needs to wrap with QueryClientProvider)
135132
useEffect(() => {
136133
if (IS_LOCAL_MODE) {
@@ -182,11 +179,7 @@ export default function MyApp({ Component, pageProps }: AppPropsWithLayout) {
182179
<HDXQueryParamProvider>
183180
<QueryParamProvider adapter={NextAdapter}>
184181
<QueryClientProvider client={queryClient}>
185-
<AppContent
186-
Component={Component}
187-
pageProps={pageProps}
188-
confirmModal={confirmModal}
189-
/>
182+
<AppContent Component={Component} pageProps={pageProps} />
190183
<ReactQueryDevtools initialIsOpen={true} />
191184
</QueryClientProvider>
192185
</QueryParamProvider>

‎packages/app/src/DBDashboardPage.tsx‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -521,10 +521,33 @@ const EditTileModal = ({
521521
}) => {
522522
const contextZIndex = useZIndex();
523523
const modalZIndex = contextZIndex + 10;
524+
const confirm = useConfirm();
525+
const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false);
526+
527+
useEffect(() => {
528+
if (chart != null) {
529+
setHasUnsavedChanges(false);
530+
}
531+
}, [chart]);
532+
533+
const handleClose = useCallback(() => {
534+
if (isSaving) return;
535+
if (hasUnsavedChanges) {
536+
confirm(
537+
'You have unsaved changes. Discard them and close the editor?',
538+
'Discard',
539+
).then(ok => {
540+
if (ok) onClose();
541+
});
542+
} else {
543+
onClose();
544+
}
545+
}, [confirm, isSaving, hasUnsavedChanges, onClose]);
546+
524547
return (
525548
<Modal
526549
opened={chart != null}
527-
onClose={onClose}
550+
onClose={handleClose}
528551
withCloseButton={false}
529552
centered
530553
size="90%"
@@ -544,7 +567,8 @@ const EditTileModal = ({
544567
config: config,
545568
});
546569
}}
547-
onClose={onClose}
570+
onClose={handleClose}
571+
onDirtyChange={setHasUnsavedChanges}
548572
/>
549573
</ZIndexContext.Provider>
550574
)}
@@ -984,9 +1008,7 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) {
9841008
dashboardId={dashboardId}
9851009
chart={editedTile}
9861010
onClose={() => {
987-
if (!isSaving) {
988-
setEditedTile(undefined);
989-
}
1011+
if (!isSaving) setEditedTile(undefined);
9901012
}}
9911013
dateRange={searchedTimeRange}
9921014
isSaving={isSaving}

‎packages/app/src/components/DBEditTimeChartForm.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,7 @@ export default function EditTimeChartForm({
573573
onSave,
574574
onTimeRangeSelect,
575575
onClose,
576+
onDirtyChange,
576577
'data-testid': dataTestId,
577578
submitRef,
578579
}: {
@@ -586,6 +587,7 @@ export default function EditTimeChartForm({
586587
setDisplayedTimeInputValue?: (value: string) => void;
587588
onSave?: (chart: SavedChartConfig) => void;
588589
onClose?: () => void;
590+
onDirtyChange?: (isDirty: boolean) => void;
589591
onTimeRangeSelect?: (start: Date, end: Date) => void;
590592
'data-testid'?: string;
591593
submitRef?: React.MutableRefObject<(() => void) | undefined>;
@@ -607,7 +609,7 @@ export default function EditTimeChartForm({
607609
register,
608610
setError,
609611
clearErrors,
610-
formState: { errors },
612+
formState: { errors, isDirty },
611613
} = useForm<SavedChartConfigWithSeries>({
612614
defaultValues: configWithSeries,
613615
values: configWithSeries,
@@ -624,6 +626,10 @@ export default function EditTimeChartForm({
624626
name: 'series',
625627
});
626628

629+
useEffect(() => {
630+
onDirtyChange?.(isDirty);
631+
}, [isDirty, onDirtyChange]);
632+
627633
const [isSampleEventsOpen, setIsSampleEventsOpen] = useState(false);
628634

629635
const select = useWatch({ control, name: 'select' });

‎packages/app/src/useConfirm.tsx‎

Lines changed: 75 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,88 +1,107 @@
11
import * as React from 'react';
2-
import { atom, useAtomValue, useSetAtom } from 'jotai';
2+
import { useRouter } from 'next/router';
33
import { Button, Group, Modal, Text } from '@mantine/core';
44

55
type ConfirmOptions = {
66
variant?: 'primary' | 'danger';
77
};
88

9-
type ConfirmAtom = {
9+
type ConfirmState = {
1010
message: React.ReactNode;
1111
confirmLabel?: string;
1212
confirmVariant?: 'primary' | 'danger';
1313
onConfirm: () => void;
14-
onClose?: () => void;
14+
onClose: () => void;
1515
} | null;
1616

17-
const confirmAtom = atom<ConfirmAtom>(null);
17+
type ConfirmFn = (
18+
message: React.ReactNode,
19+
confirmLabel?: string,
20+
options?: ConfirmOptions,
21+
) => Promise<boolean>;
1822

19-
export const useConfirm = () => {
20-
const setConfirm = useSetAtom(confirmAtom);
23+
const ConfirmContext = React.createContext<ConfirmFn | null>(null);
24+
25+
export function ConfirmProvider({ children }: { children: React.ReactNode }) {
26+
const [state, setState] = React.useState<ConfirmState>(null);
27+
const router = useRouter();
28+
29+
// Keep a ref so the route-change handler always sees the latest state
30+
// without needing to be re-registered on every render.
31+
const stateRef = React.useRef(state);
32+
React.useEffect(() => {
33+
stateRef.current = state;
34+
}, [state]);
2135

22-
return React.useCallback(
23-
async (
24-
message: React.ReactNode,
25-
confirmLabel?: string,
26-
options?: ConfirmOptions,
27-
): Promise<boolean> => {
28-
return new Promise(resolve => {
29-
setConfirm({
36+
React.useEffect(() => {
37+
const dismiss = () => {
38+
stateRef.current?.onClose();
39+
};
40+
router.events.on('routeChangeStart', dismiss);
41+
return () => router.events.off('routeChangeStart', dismiss);
42+
}, [router.events]);
43+
44+
const confirm = React.useCallback<ConfirmFn>(
45+
(message, confirmLabel, options) => {
46+
return new Promise<boolean>(resolve => {
47+
setState({
3048
message,
3149
confirmLabel,
3250
confirmVariant: options?.variant ?? 'primary',
3351
onConfirm: () => {
3452
resolve(true);
35-
setConfirm(null);
53+
setState(null);
3654
},
3755
onClose: () => {
3856
resolve(false);
39-
setConfirm(null);
57+
setState(null);
4058
},
4159
});
4260
});
4361
},
44-
[setConfirm],
62+
[],
4563
);
46-
};
47-
48-
export const useConfirmModal = () => {
49-
const confirm = useAtomValue(confirmAtom);
50-
const setConfirm = useSetAtom(confirmAtom);
51-
52-
const handleClose = React.useCallback(() => {
53-
confirm?.onClose?.();
54-
setConfirm(null);
55-
}, [confirm, setConfirm]);
5664

5765
return (
58-
<Modal
59-
data-testid="confirm-modal"
60-
opened={!!confirm}
61-
onClose={handleClose}
62-
centered
63-
withCloseButton={false}
64-
>
65-
<Text size="sm" opacity={0.7}>
66-
{confirm?.message}
67-
</Text>
68-
<Group justify="flex-end" mt="md" gap="xs">
69-
<Button
70-
data-testid="confirm-cancel-button"
71-
size="xs"
72-
variant="secondary"
73-
onClick={handleClose}
74-
>
75-
Cancel
76-
</Button>
77-
<Button
78-
data-testid="confirm-confirm-button"
79-
size="xs"
80-
variant={confirm?.confirmVariant ?? 'primary'}
81-
onClick={confirm?.onConfirm}
82-
>
83-
{confirm?.confirmLabel || 'Confirm'}
84-
</Button>
85-
</Group>
86-
</Modal>
66+
<ConfirmContext.Provider value={confirm}>
67+
{children}
68+
<Modal
69+
data-testid="confirm-modal"
70+
opened={!!state}
71+
onClose={state?.onClose ?? (() => {})}
72+
centered
73+
withCloseButton={false}
74+
>
75+
<Text size="sm" opacity={0.7}>
76+
{state?.message}
77+
</Text>
78+
<Group justify="flex-end" mt="md" gap="xs">
79+
<Button
80+
data-testid="confirm-cancel-button"
81+
size="xs"
82+
variant="secondary"
83+
onClick={state?.onClose}
84+
>
85+
Cancel
86+
</Button>
87+
<Button
88+
data-testid="confirm-confirm-button"
89+
size="xs"
90+
variant={state?.confirmVariant ?? 'primary'}
91+
onClick={state?.onConfirm}
92+
>
93+
{state?.confirmLabel || 'Confirm'}
94+
</Button>
95+
</Group>
96+
</Modal>
97+
</ConfirmContext.Provider>
8798
);
99+
}
100+
101+
export const useConfirm = () => {
102+
const confirm = React.useContext(ConfirmContext);
103+
if (confirm == null) {
104+
throw new Error('useConfirm must be used within a ConfirmProvider');
105+
}
106+
return confirm;
88107
};

‎packages/app/tests/e2e/README.md‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,20 @@ yarn test:e2e tests/e2e/features/search/search.spec.ts
7474
yarn test:e2e tests/e2e/features/dashboard.spec.ts --local
7575
```
7676

77+
**Watch mode (re-run on file save):**
78+
79+
Playwright UI has built-in watch. Run with UI, then enable it per test:
80+
81+
```bash
82+
./scripts/test-e2e.sh --keep-running --ui
83+
```
84+
85+
In the Playwright UI sidebar, click the **eye icon** next to a test (or file/describe) to turn on watch for it. When you save changes to that test file, that test will re-run automatically.
86+
7787
**Available flags:**
7888

7989
- `--local` - Run in local mode (frontend only), excludes `@full-stack` tests
80-
- `--ui` - Open Playwright UI for interactive debugging
90+
- `--ui` - Open Playwright UI for interactive debugging and watch mode
8191
- `--debug` - Run in debug mode with browser developer tools
8292
- `--headed` - Run tests in visible browser (default is headless)
8393

‎packages/app/tests/e2e/features/dashboard.spec.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,41 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
257257
},
258258
);
259259

260+
test('should warn when closing tile editor with unsaved changes', async () => {
261+
await dashboardPage.openNewTileEditor();
262+
await dashboardPage.chartEditor.setChartName('My Unsaved Chart');
263+
264+
await dashboardPage.page.keyboard.press('Escape');
265+
await expect(dashboardPage.unsavedChangesConfirmModal).toBeAttached({
266+
timeout: 5000,
267+
});
268+
269+
await dashboardPage.unsavedChangesConfirmCancelButton.click();
270+
await expect(dashboardPage.unsavedChangesConfirmModal).toBeHidden();
271+
await expect(dashboardPage.chartEditor.nameInput).toHaveValue(
272+
'My Unsaved Chart',
273+
);
274+
275+
await dashboardPage.page.keyboard.press('Escape');
276+
await expect(dashboardPage.unsavedChangesConfirmModal).toBeAttached({
277+
timeout: 5000,
278+
});
279+
await dashboardPage.unsavedChangesConfirmDiscardButton.click();
280+
await expect(dashboardPage.chartEditor.nameInput).toBeHidden({
281+
timeout: 5000,
282+
});
283+
});
284+
285+
test('should close tile editor without confirm when there are no unsaved changes', async () => {
286+
await dashboardPage.openNewTileEditor();
287+
288+
await dashboardPage.page.keyboard.press('Escape');
289+
await expect(dashboardPage.chartEditor.nameInput).toBeHidden({
290+
timeout: 5000,
291+
});
292+
await expect(dashboardPage.unsavedChangesConfirmModal).toBeHidden();
293+
});
294+
260295
test(
261296
'should create and populate filters',
262297
{ tag: '@full-stack' },

0 commit comments

Comments
 (0)