Update plugin action modals (#96880)

* Update plugin action modals * Fix styles for Jetpack Cloud * Revert heading wording for multiple plugins * Update unit tests * Fix unit test * Fix tests * use Modal component for plugin action dialogs * Revert "use Modal component for plugin action dialogs" This reverts commit b4de867. * fix CSS leakage --------- Co-authored-by: Gergely Csécsey <[email protected]>
Automattic · Dec 24, 2024 · 7c98dc2 · 7c98dc2
1 parent 3207ba4
commit 7c98dc2
Show file tree

Hide file tree

Showing 19 changed files with 222 additions and 134 deletions.
diff --git a/client/assets/stylesheets/shared/functions/_z-index.scss b/client/assets/stylesheets/shared/functions/_z-index.scss
@@ -154,6 +154,9 @@ $z-layers: (
 		".theme-preview-modal": 100200,
 		".wplink__dialog.dialog.card": 100200,
 		".web-preview": 100200,
+		// TODO: remove after /plugins/manage details doesn't use Dialog
+		".is-section-plugins .components-modal__screen-overlay": 100201,
+		".is-section-jetpack-cloud-plugin-management .components-modal__screen-overlay": 100201,
 		".theme-site-selector-modal": 100201,
 		".is-section-theme .global-notices": 100201,
 		".popover.ellipsis-menu__menu": 100201,

diff --git a/client/lib/accept/dialog.jsx b/client/lib/accept/dialog.jsx
@@ -1,4 +1,5 @@
 import { Dialog } from '@automattic/components';
+import { Button, Modal } from '@wordpress/components';
 import clsx from 'clsx';
 import { localize } from 'i18n-calypso';
 import PropTypes from 'prop-types';
@@ -12,10 +13,16 @@ class AcceptDialog extends Component {
 	static propTypes = {
 		translate: PropTypes.func,
 		message: PropTypes.node,
+		title: PropTypes.node,
 		onClose: PropTypes.func.isRequired,
 		confirmButtonText: PropTypes.node,
 		cancelButtonText: PropTypes.node,
-		options: PropTypes.object,
+		options: PropTypes.shape( {
+			isScary: PropTypes.bool,
+			additionalClassNames: PropTypes.string,
+			useModal: PropTypes.bool,
+			modalOptions: PropTypes.object,
+		} ),
 	};
 
 	state = { isVisible: true };
@@ -52,7 +59,33 @@ class AcceptDialog extends Component {
 		if ( ! this.state.isVisible ) {
 			return null;
 		}
-
+		if ( this.props.options?.useModal ) {
+			return (
+				<Modal
+					title={ this.props.options?.modalOptions?.title }
+					onRequestClose={ this.onClose }
+					className={ clsx(
+						'accept__dialog-use-modal',
+						this.props?.options?.additionalClassNames
+					) }
+					size="medium"
+				>
+					{ this.props.message }
+					<div className="accept__dialog-buttons">
+						{ this.getActionButtons().map( ( button ) => (
+							<Button
+								key={ button.action }
+								onClick={ () => this.onClose( button.action ) }
+								variant={ button.isPrimary ? 'primary' : 'secondary' }
+								className={ button.additionalClassNames }
+							>
+								{ button.label }
+							</Button>
+						) ) }
+					</div>
+				</Modal>
+			);
+		}
 		return (
 			<Dialog
 				buttons={ this.getActionButtons() }

diff --git a/client/lib/accept/dialog.scss b/client/lib/accept/dialog.scss
@@ -1,4 +1,45 @@
+@import '@automattic/typography/styles/fonts';
+
 .accept__dialog {
 	padding-bottom: 24px;
 	max-width: 500px;
 }
+
+.accept__dialog-use-modal {
+	.components-modal__header h1.components-modal__header-heading {
+		font-family: $sans;
+		font-size: 1.25rem;
+		font-weight: 600;
+	}
+
+	.accept__dialog-buttons {
+		display: flex;
+		justify-content: end;
+		gap: 0.5rem;
+		margin-top: 1.5rem;
+
+		.components-button.is-primary.is-scary {
+			background-color: var( --color-error );
+			color: var( --color-text-inverted );
+			border-color: var( --color-error );
+
+			&:hover,
+			&:focus-visible {
+				background-color: var( --color-error-60 );
+			}
+		}
+	}
+}
+
+// TODO: remove after /plugins/manage details doesn't use Dialog
+.is-section-plugins .components-modal__screen-overlay {
+	z-index: z-index( 'root', '.is-section-plugins .components-modal__screen-overlay' );
+}
+
+// TODO: remove after /plugins/manage details doesn't use Dialog
+.is-section-jetpack-cloud-plugin-management .components-modal__screen-overlay {
+	z-index: z-index(
+		'root',
+		'.is-section-jetpack-cloud-plugin-management .components-modal__screen-overlay'
+	);
+}
diff --git a/client/my-sites/plugins/hooks/test/use-show-plugin-action-dialog.jsx b/client/my-sites/plugins/hooks/test/use-show-plugin-action-dialog.jsx
@@ -1,14 +1,23 @@
 /**
  * @jest-environment jsdom
  */
-import { render, renderHook } from '@testing-library/react';
+import { render, renderHook, cleanup } from '@testing-library/react';
 import { PluginActions } from '../types';
 import useShowPluginActionDialog from '../use-show-plugin-action-dialog';
 
 const HEADING_TEXT = 'Heading';
 const MESSAGE_TEXT = 'Message';
+const CONFIRM_TEXT = 'OK';
+const CANCEL_TEXT = 'Cancel';
 jest.mock( '../use-get-dialog-text', () =>
-	jest.fn().mockReturnValue( () => ( { heading: HEADING_TEXT, message: MESSAGE_TEXT } ) )
+	jest.fn().mockReturnValue( () => ( {
+		heading: HEADING_TEXT,
+		message: MESSAGE_TEXT,
+		cta: {
+			confirm: CONFIRM_TEXT,
+			cancel: CANCEL_TEXT,
+		},
+	} ) )
 );
 
 const runHook = () => renderHook( () => useShowPluginActionDialog() ).result.current;
@@ -17,8 +26,8 @@ describe( 'useShowPluginActionDialog', () => {
 	// A new dialog is created every time we call showPluginActionDialog, and
 	// JSDOM doesn't clear the page before each test; so, we have to clear the
 	// document ourselves.
-	beforeEach( () => {
-		document.documentElement.innerHTML = '<head></head><body></body>';
+	afterEach( () => {
+		cleanup();
 	} );
 
 	it( 'renders a dialog modal', () => {
@@ -31,7 +40,7 @@ describe( 'useShowPluginActionDialog', () => {
 		expect( result.queryByRole( 'dialog' ) ).toBeInTheDocument();
 	} );
 
-	it( 'displays the correct heading and message text', () => {
+	it( 'displays the correct message text', () => {
 		const showPluginActionDialog = runHook();
 
 		const callback = () => {
@@ -41,10 +50,10 @@ describe( 'useShowPluginActionDialog', () => {
 
 		// NOTE: Selecting these elements by class is less than ideal,
 		// but currently there's no other way to reliably identify them
-		const heading = result.getByText( HEADING_TEXT, { selector: 'div' } );
+		const heading = result.getByText( HEADING_TEXT, { selector: 'h1' } );
 		expect( heading ).toBeInTheDocument();
 
-		const message = result.getByText( MESSAGE_TEXT, { selector: 'span' } );
+		const message = result.getByText( MESSAGE_TEXT, { selector: 'p' } );
 		expect( message ).toBeInTheDocument();
 	} );
 
@@ -55,7 +64,7 @@ describe( 'useShowPluginActionDialog', () => {
 			/* Purposely do nothing */
 		};
 		const result = render( showPluginActionDialog( PluginActions.REMOVE, [], [], callback ) );
-		const button = result.getByRole( 'button', { name: HEADING_TEXT } );
+		const button = result.getByRole( 'button', { name: CONFIRM_TEXT } );
 		expect( button.classList ).toContain( 'is-scary' );
 	} );
 
@@ -65,7 +74,7 @@ describe( 'useShowPluginActionDialog', () => {
 		const callback = jest.fn();
 
 		const result = render( showPluginActionDialog( PluginActions.REMOVE, [], [], callback ) );
-		const acceptButton = result.getByRole( 'button', { name: HEADING_TEXT } );
+		const acceptButton = result.getByRole( 'button', { name: CONFIRM_TEXT } );
 		acceptButton.click();
 
 		expect( callback ).toHaveBeenCalledWith( true );
@@ -77,7 +86,7 @@ describe( 'useShowPluginActionDialog', () => {
 		const callback = jest.fn();
 
 		const result = render( showPluginActionDialog( PluginActions.REMOVE, [], [], callback ) );
-		const cancelButton = result.getByRole( 'button', { name: 'Cancel' } );
+		const cancelButton = result.getByRole( 'button', { name: CANCEL_TEXT } );
 		cancelButton.click();
 
 		expect( callback ).toHaveBeenCalledWith( false );

diff --git a/client/my-sites/plugins/hooks/use-get-dialog-text/actions/activate.ts b/client/my-sites/plugins/hooks/use-get-dialog-text/actions/activate.ts
@@ -1,10 +1,8 @@
+import { translate } from 'i18n-calypso';
 import type { ActionTexts, ActionHeadings, ActionMessages } from '../types';
 
 const headings: ActionHeadings = {
-	onePlugin: ( plugin ) => ( translate ) =>
-		translate( 'Activate %(plugin)s', {
-			args: { plugin: plugin.name ?? plugin.slug },
-		} ),
+	onePlugin: ( translate ) => translate( 'Activate plugin' ),
 	manyPlugins: ( plugins ) => ( translate ) =>
 		translate( 'Activate %(pluginCount)d plugin', 'Activate %(pluginCount)d plugins', {
 			count: plugins.length,
@@ -74,9 +72,15 @@ const messages: ActionMessages = {
 	},
 };
 
+const cta = {
+	confirm: translate( 'Activate' ),
+	cancel: translate( 'Cancel' ),
+};
+
 const actionTexts: ActionTexts = {
 	headings,
 	messages,
+	cta,
 };
 
 export default actionTexts;
diff --git a/client/my-sites/plugins/hooks/use-get-dialog-text/actions/deactivate.ts b/client/my-sites/plugins/hooks/use-get-dialog-text/actions/deactivate.ts
@@ -1,10 +1,8 @@
+import { translate } from 'i18n-calypso';
 import type { ActionTexts, ActionHeadings, ActionMessages } from '../types';
 
 const headings: ActionHeadings = {
-	onePlugin: ( plugin ) => ( translate ) =>
-		translate( 'Deactivate %(plugin)s', {
-			args: { plugin: plugin.name ?? plugin.slug },
-		} ),
+	onePlugin: ( translate ) => translate( 'Deactivate plugin' ),
 	manyPlugins: ( plugins ) => ( translate ) =>
 		translate( 'Deactivate %(pluginCount)d plugin', 'Deactivate %(pluginCount)d plugins', {
 			count: plugins.length,
@@ -74,9 +72,15 @@ const messages: ActionMessages = {
 	},
 };
 
+const cta = {
+	confirm: translate( 'Deactivate' ),
+	cancel: translate( 'Cancel' ),
+};
+
 const actionTexts: ActionTexts = {
 	headings,
 	messages,
+	cta,
 };
 
 export default actionTexts;
diff --git a/client/my-sites/plugins/hooks/use-get-dialog-text/actions/disable-autoupdates.ts b/client/my-sites/plugins/hooks/use-get-dialog-text/actions/disable-autoupdates.ts
@@ -1,14 +1,8 @@
+import { translate } from 'i18n-calypso';
 import type { ActionTexts, ActionHeadings, ActionMessages } from '../types';
 
 const headings: ActionHeadings = {
-	onePlugin: ( plugin ) => ( translate ) =>
-		translate(
-			'Disable auto-updates for %(plugin)s',
-			'Disable auto-updates for %(pluginCount)d plugins',
-			{
-				args: { plugin: plugin.name ?? plugin.slug },
-			}
-		),
+	onePlugin: ( translate ) => translate( 'Disable auto-updates for plugin' ),
 	manyPlugins: ( plugins ) => ( translate ) =>
 		translate(
 			'Disable auto-updates for %(pluginCount)d plugin',
@@ -85,9 +79,15 @@ const messages: ActionMessages = {
 	},
 };
 
+const cta = {
+	confirm: translate( 'Disable' ),
+	cancel: translate( 'Cancel' ),
+};
+
 const actionTexts: ActionTexts = {
 	headings,
 	messages,
+	cta,
 };
 
 export default actionTexts;
diff --git a/client/my-sites/plugins/hooks/use-get-dialog-text/actions/enable-autoupdates.ts b/client/my-sites/plugins/hooks/use-get-dialog-text/actions/enable-autoupdates.ts
@@ -1,14 +1,8 @@
+import { translate } from 'i18n-calypso';
 import type { ActionTexts, ActionHeadings, ActionMessages } from '../types';
 
 const headings: ActionHeadings = {
-	onePlugin: ( plugin ) => ( translate ) =>
-		translate(
-			'Enable auto-updates for %(plugin)s',
-			'Enable auto-updates for %(pluginCount)d plugins',
-			{
-				args: { plugin: plugin.name ?? plugin.slug },
-			}
-		),
+	onePlugin: ( translate ) => translate( 'Enable auto-updates for plugin' ),
 	manyPlugins: ( plugins ) => ( translate ) =>
 		translate(
 			'Enable auto-updates for %(pluginCount)d plugin',
@@ -85,9 +79,15 @@ const messages: ActionMessages = {
 	},
 };
 
+const cta = {
+	confirm: translate( 'Enable' ),
+	cancel: translate( 'Cancel' ),
+};
+
 const actionTexts: ActionTexts = {
 	headings,
 	messages,
+	cta,
 };
 
 export default actionTexts;
diff --git a/client/my-sites/plugins/hooks/use-get-dialog-text/actions/remove.ts b/client/my-sites/plugins/hooks/use-get-dialog-text/actions/remove.ts
@@ -1,10 +1,8 @@
+import { translate } from 'i18n-calypso';
 import type { ActionTexts, ActionHeadings, ActionMessages } from '../types';
 
 const headings: ActionHeadings = {
-	onePlugin: ( plugin ) => ( translate ) =>
-		translate( 'Deactivate and remove %(plugin)s', {
-			args: { plugin: plugin.name ?? plugin.slug },
-		} ),
+	onePlugin: ( translate ) => translate( 'Deactivate and remove plugin' ),
 	manyPlugins: ( plugins ) => ( translate ) =>
 		translate(
 			'Deactivate and remove %(pluginCount)d plugin',
@@ -81,9 +79,15 @@ const messages: ActionMessages = {
 	},
 };
 
+const cta = {
+	confirm: translate( 'Deactivate and remove' ),
+	cancel: translate( 'Cancel' ),
+};
+
 const actionTexts: ActionTexts = {
 	headings,
 	messages,
+	cta,
 };
 
 export default actionTexts;
diff --git a/client/my-sites/plugins/hooks/use-get-dialog-text/actions/unspecified-action.ts b/client/my-sites/plugins/hooks/use-get-dialog-text/actions/unspecified-action.ts
@@ -1,10 +1,8 @@
+import { translate } from 'i18n-calypso';
 import type { ActionTexts, ActionHeadings, ActionMessages } from '../types';
 
 const headings: ActionHeadings = {
-	onePlugin: ( plugin ) => ( translate ) =>
-		translate( 'Affect %(plugin)s', {
-			args: { plugin: plugin.name ?? plugin.slug },
-		} ),
+	onePlugin: ( translate ) => translate( 'Affect plugin' ),
 	manyPlugins: ( plugins ) => ( translate ) =>
 		translate( 'Affect %(pluginCount)d plugin', 'Affect %(pluginCount)d plugins', {
 			count: plugins.length,
@@ -74,9 +72,15 @@ const messages: ActionMessages = {
 	},
 };
 
+const cta = {
+	confirm: translate( 'OK' ),
+	cancel: translate( 'Cancel' ),
+};
+
 const actionTexts: ActionTexts = {
 	headings,
 	messages,
+	cta,
 };
 
 export default actionTexts;