Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion apps/admin-x-framework/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@
"vitest": "catalog:"
},
"dependencies": {
"@ebay/nice-modal-react": "catalog:",
"@sentry/react": "catalog:",
"@tanstack/react-query": "catalog:",
"@tinybirdco/charts": "0.3.0",
Expand Down
1 change: 0 additions & 1 deletion apps/admin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
"@codemirror/state": "catalog:",
"@codemirror/theme-one-dark": "catalog:",
"@dnd-kit/sortable": "catalog:",
"@ebay/nice-modal-react": "catalog:",
"@sentry/react": "catalog:",
"@svg-maps/world": "2.0.0",
"@tanstack/react-query": "catalog:",
Expand Down
9 changes: 3 additions & 6 deletions apps/admin/src/settings/app/app.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import MainContent from './main-content';
import NiceModal from '@ebay/nice-modal-react';
import SettingsAppProvider, {type UpgradeStatusType} from './components/providers/settings-app-provider';
import {ConfirmationProvider} from './components/providers/confirmation-provider';
import {DialogPortalProvider} from './components/providers/dialog-portal';
Expand Down Expand Up @@ -31,11 +30,9 @@ export function App({upgradeStatus}: AppProps) {
<div className='admin-x-base admin-x-settings [--color-focus-ring:var(--color-green-500)] [--focus-ring:var(--color-green-500)]'>
<ConfirmationProvider>
<DialogPortalProvider>
<NiceModal.Provider>
<SettingsLocationSync />
<MainContent />
<Outlet />
</NiceModal.Provider>
<SettingsLocationSync />
<MainContent />
<Outlet />
</DialogPortalProvider>
</ConfirmationProvider>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ export const ConfirmationProvider: React.FC<{children: React.ReactNode}> = ({chi
const show = useCallback((request: Omit<ConfirmationRequest, 'id'>): ConfirmationHandle => {
nextId.current += 1;
const id = nextId.current;
// One request per kind, matching NiceModal's per-component keying: a
// second show replaces the first instead of stacking (StrictMode
// double-effects depend on this).
// One request per kind: a second show replaces the first instead of
// stacking (StrictMode double-effects depend on this).
setRequests(current => [...current.filter(r => r.kind !== request.kind), {...request, id} as ConfirmationRequest]);
return {remove: () => setRequests(current => current.filter(r => r.id !== id))};
}, []);
Expand Down
30 changes: 7 additions & 23 deletions apps/admin/src/settings/app/components/settings/preview-modal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, {useEffect} from 'react';
import {ExternalLink} from 'lucide-react';
import {useModal} from '@ebay/nice-modal-react';

import {Box, Inline, Text, type TextElement, type TextLeading, type TextSize} from '@tryghost/shade/primitives';
import {Button, type ButtonProps, Separator} from '@tryghost/shade/components';
Expand Down Expand Up @@ -37,8 +36,8 @@ const headingLeading: Record<HeadingLevel, TextLeading> = {
};

/**
* Compatibility shell for settings preview modals while the legacy NiceModal
* flows are migrated to consumer-controlled Shade compositions.
* Consumer-controlled shell for the settings preview dialogs (design, portal,
* newsletters, offers). New modal flows should use Shade compositions directly.
*/
export interface PreviewModalProps {
testId?: string;
Expand Down Expand Up @@ -72,15 +71,12 @@ export interface PreviewModalProps {

onCancel?: () => void;
onOk?: () => void;
/** Supersedes the NiceModal close path; without it the modal must be mounted through NiceModal. Keep its presence stable across renders — toggling defined/undefined remounts the modal subtree. */
onClose?: () => void;
onClose: () => void;
afterClose?: () => void;
}

type PreviewModalContentBaseProps = Omit<PreviewModalProps, 'onClose'> & {requestClose: () => void};

const PreviewModalContentBase: React.FC<PreviewModalContentBaseProps> = ({
requestClose,
export const PreviewModalContent: React.FC<PreviewModalProps> = ({
onClose,
testId,
title,
titleHeadingLevel = 4,
Expand Down Expand Up @@ -188,7 +184,7 @@ const PreviewModalContentBase: React.FC<PreviewModalContentBaseProps> = ({

const handleCancel = onCancel || (() => {
confirm(dirty, () => {
requestClose();
onClose();
afterClose?.();
});
});
Expand All @@ -207,7 +203,7 @@ const PreviewModalContentBase: React.FC<PreviewModalContentBaseProps> = ({
title=''
width={width}
hideXOnMobile
onClose={requestClose}
onClose={onClose}
>
<Inline align='stretch' className='h-full grow' gap='none'>
<Box className={cn(
Expand Down Expand Up @@ -253,15 +249,3 @@ const PreviewModalContentBase: React.FC<PreviewModalContentBaseProps> = ({
</SettingsModal>
);
};

const NicePreviewModalContent: React.FC<Omit<PreviewModalContentBaseProps, 'requestClose'>> = (props) => {
const modal = useModal();
return <PreviewModalContentBase {...props} requestClose={() => modal.remove()} />;
};

export const PreviewModalContent: React.FC<PreviewModalProps> = ({onClose, ...props}) => {
if (onClose) {
return <PreviewModalContentBase {...props} requestClose={onClose} />;
}
return <NicePreviewModalContent {...props} />;
};
1 change: 0 additions & 1 deletion apps/shade/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@
"@dnd-kit/core": "catalog:",
"@dnd-kit/sortable": "catalog:",
"@dnd-kit/utilities": "catalog:",
"@ebay/nice-modal-react": "catalog:",
"@hookform/resolvers": "5.4.0",
"@number-flow/react": "0.6.2",
"@radix-ui/react-accordion": "1.2.15",
Expand Down
18 changes: 7 additions & 11 deletions apps/shade/src/components/patterns/settings-modal.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import NiceModal from '@ebay/nice-modal-react';
import {useState} from 'react';
import type {Meta, StoryObj} from '@storybook/react-vite';

import {Button} from '@/components/ui/button';
import {Box} from '@/components/primitives/box';
import {SettingsModal, type SettingsModalProps} from '@/components/patterns/settings-modal';

const SettingsModalStory = (props: SettingsModalProps) => {
const StoryModal = NiceModal.create<SettingsModalProps>(() => <SettingsModal {...props} />);
const SettingsModalStory = (props: Omit<SettingsModalProps, 'onClose'>) => {
const [open, setOpen] = useState(false);

return (
<Box className='min-h-72 bg-background p-8'>
<Button onClick={() => NiceModal.show(StoryModal)}>Open modal</Button>
<Button onClick={() => setOpen(true)}>Open modal</Button>
{open && <SettingsModal {...props} onClose={() => setOpen(false)} />}
</Box>
);
};
Expand All @@ -22,15 +23,10 @@ const meta = {
parameters: {
docs: {
description: {
component: 'Transitional compatibility shell for the existing settings NiceModal flows. New modal flows should use Shade Dialog primitives directly.'
component: 'Consumer-controlled shell for the legacy full-page settings dialogs. New modal flows should use Shade Dialog primitives directly.'
}
}
},
decorators: [Story => (
<NiceModal.Provider>
<Story />
</NiceModal.Provider>
)]
}
} satisfies Meta<typeof SettingsModalStory>;

export default meta;
Expand Down
32 changes: 6 additions & 26 deletions apps/shade/src/components/patterns/settings-modal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {useModal} from '@ebay/nice-modal-react';
import {cva} from 'class-variance-authority';
import {X} from 'lucide-react';
import React, {forwardRef, useEffect, useState} from 'react';
Expand All @@ -14,8 +13,8 @@ import useGlobalDirtyState from '@/hooks/use-global-dirty-state';
import {cn} from '@/lib/utils';

/**
* Compatibility shell for settings modals while the legacy NiceModal flows are
* migrated to Shade's consumer-controlled Dialog primitives.
* Consumer-controlled settings modal shell. Legacy full-page settings dialogs
* still render through it; new modal flows should use Shade Dialog primitives.
*/
export type SettingsModalSize = 'sm' | 'md' | 'lg' | 'xl' | 'full' | 'bleed';

Expand Down Expand Up @@ -44,8 +43,7 @@ export interface SettingsModalProps {
onCancel?: () => void;
topRightContent?: 'close' | React.ReactNode;
hideXOnMobile?: boolean;
/** Supersedes the NiceModal close path; without it the modal must be mounted through NiceModal. Keep its presence stable across renders — toggling defined/undefined remounts the modal subtree. */
onClose?: () => void;
onClose: () => void;
afterClose?: () => void;
children?: React.ReactNode;
backDrop?: boolean;
Expand Down Expand Up @@ -124,9 +122,7 @@ const headerOffsets: Record<SettingsModalSize, string> = {
bleed: '-inset-x-10'
};

type SettingsModalContentProps = Omit<SettingsModalProps, 'onClose'> & {requestClose: () => void};

const SettingsModalContent = forwardRef<HTMLElement, SettingsModalContentProps>(({
const SettingsModal = forwardRef<HTMLElement, SettingsModalProps>(({
'aria-label': ariaLabel,
className,
size = 'md',
Expand All @@ -150,7 +146,7 @@ const SettingsModalContent = forwardRef<HTMLElement, SettingsModalContentProps>(
onCancel,
topRightContent,
hideXOnMobile = false,
requestClose,
onClose,
afterClose,
children,
backDrop = true,
Expand All @@ -173,7 +169,7 @@ const SettingsModalContent = forwardRef<HTMLElement, SettingsModalContentProps>(

const removeModal = () => {
confirm(dirty, () => {
requestClose();
onClose();
afterClose?.();
});
};
Expand Down Expand Up @@ -359,22 +355,6 @@ const SettingsModalContent = forwardRef<HTMLElement, SettingsModalContentProps>(
);
});

SettingsModalContent.displayName = 'SettingsModalContent';

const NiceSettingsModal = forwardRef<HTMLElement, Omit<SettingsModalContentProps, 'requestClose'>>((props, ref) => {
const modal = useModal();
return <SettingsModalContent ref={ref} {...props} requestClose={() => modal.remove()} />;
});

NiceSettingsModal.displayName = 'NiceSettingsModal';

const SettingsModal = forwardRef<HTMLElement, SettingsModalProps>(({onClose, ...props}, ref) => {
if (onClose) {
return <SettingsModalContent ref={ref} {...props} requestClose={onClose} />;
}
return <NiceSettingsModal ref={ref} {...props} />;
});

SettingsModal.displayName = 'SettingsModal';

export {SettingsModal, settingsModalVariants};
48 changes: 9 additions & 39 deletions apps/shade/test/unit/components/patterns/settings-modal.test.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
import NiceModal from '@ebay/nice-modal-react';
import {act, fireEvent, render, screen, waitFor} from '@testing-library/react';
import {fireEvent, render, screen, waitFor} from '@testing-library/react';
import {describe, expect, it, vi} from 'vitest';

import {SettingsModal, settingsModalVariants, type SettingsModalSize} from '@/components/patterns/settings-modal';

const TestSettingsModal = NiceModal.create(() => (
<SettingsModal title='Test modal' onOk={() => undefined}>
Modal content
</SettingsModal>
));

describe('SettingsModal', () => {
it.each<SettingsModalSize>(['sm', 'md', 'lg', 'xl', 'full'])('uses the standard dialog radius for the %s size', (size) => {
expect(settingsModalVariants({size})).toContain('rounded-lg');
Expand All @@ -21,27 +14,23 @@ describe('SettingsModal', () => {
expect(settingsModalVariants({size})).not.toContain('rounded-lg');
});

it('uses content-sized outline and primary actions by default', async () => {
render(<NiceModal.Provider />);

act(() => {
void NiceModal.show(TestSettingsModal);
});
it('uses content-sized outline and primary actions by default', () => {
render(
<SettingsModal title='Test modal' onClose={() => undefined} onOk={() => undefined}>
Modal content
</SettingsModal>
);

const cancelButton = await screen.findByRole('button', {name: 'Cancel'});
const cancelButton = screen.getByRole('button', {name: 'Cancel'});
const okButton = screen.getByRole('button', {name: 'OK'});

expect(cancelButton.className).toContain('border-control-border');
expect(cancelButton.className).toContain('bg-transparent');
expect(cancelButton.className).not.toContain('hover:bg-accent');
expect(okButton.className).not.toContain('min-w-20');

act(() => {
void NiceModal.remove(TestSettingsModal);
});
});

it('renders without a NiceModal context and closes through onClose', () => {
it('closes through onClose', () => {
const onClose = vi.fn();
render(
<SettingsModal title='Test modal' topRightContent='close' onClose={onClose}>
Expand Down Expand Up @@ -71,23 +60,4 @@ describe('SettingsModal', () => {
expect(onClose).toHaveBeenCalledTimes(1);
});
});

it('still closes through NiceModal when no onClose is passed', async () => {
const BridgeModal = NiceModal.create(() => (
<SettingsModal title='Bridge modal' topRightContent='close'>
Modal content
</SettingsModal>
));

render(<NiceModal.Provider />);
act(() => {
void NiceModal.show(BridgeModal);
});

fireEvent.click(await screen.findByTestId('close-modal'));

await waitFor(() => {
expect(screen.queryByTestId('close-modal')).toBeNull();
});
});
});
Loading
Loading