From b1270ca23f8231e58a905244d1fbca4d7a026caa Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Fri, 2 Aug 2019 13:53:19 +1000 Subject: [PATCH] InlineTip: Use Modal instead of window.confirm() --- packages/components/src/notice/index.js | 11 ++- packages/e2e-tests/specs/nux.test.js | 29 +++---- .../nux/src/components/inline-tip/index.js | 78 ++++++++++++++----- .../nux/src/components/inline-tip/style.scss | 13 ++++ .../test/__snapshots__/index.js.snap | 34 ++------ .../src/components/inline-tip/test/index.js | 44 ++++++----- packages/nux/src/style.scss | 1 + 7 files changed, 124 insertions(+), 86 deletions(-) create mode 100644 packages/nux/src/components/inline-tip/style.scss diff --git a/packages/components/src/notice/index.js b/packages/components/src/notice/index.js index 99d5f9968f1111..6a945cd8c7e509 100644 --- a/packages/components/src/notice/index.js +++ b/packages/components/src/notice/index.js @@ -24,9 +24,14 @@ function Notice( { actions = [], __unstableHTML, } ) { - const classes = classnames( className, 'components-notice', 'is-' + status, { - 'is-dismissible': isDismissible, - } ); + const classes = classnames( + className, + 'components-notice', + status ? 'is-' + status : undefined, + { + 'is-dismissible': isDismissible, + } + ); if ( __unstableHTML ) { children = { children }; diff --git a/packages/e2e-tests/specs/nux.test.js b/packages/e2e-tests/specs/nux.test.js index 4d1620648565c9..f5f5c7e355aa28 100644 --- a/packages/e2e-tests/specs/nux.test.js +++ b/packages/e2e-tests/specs/nux.test.js @@ -40,43 +40,36 @@ describe( 'New User Experience (NUX)', () => { expect( blockInspectorTip ).not.toBeNull(); } ); - it( 'should dismiss a single tip if X button is clicked and dialog is dismissed', async () => { - // We need to *dismiss* the upcoming confirm() dialog, so let's temporarily - // remove the listener that was added in by enablePageDialogAccept(). - const listeners = page.rawListeners( 'dialog' ); - page.removeAllListeners( 'dialog' ); - + it( 'should dismiss a single tip if X button is clicked and confirmation is dismissed', async () => { // Open up the inserter. await openGlobalBlockInserter(); - // Dismiss the upcoming confirm() dialog. - page.once( 'dialog', async ( dialog ) => { - await dialog.dismiss(); - } ); - // Click the tip's X button. await page.click( '.block-editor-inserter__tip button[aria-label="Dismiss this notice"]' ); + // Dismiss the confirmation modal. + const [ noButton ] = await page.$x( '//*[contains(@class, "nux-hide-tips-confirmation")]//button[text()="No"]' ); + await noButton.click(); + // The tip should be gone. const inserterTip = await page.$( '.block-editor-inserter__tip' ); expect( inserterTip ).toBeNull(); // Tips should still be enabled. expect( await areTipsEnabled() ).toBe( true ); - - // Restore the listeners that we removed above. - for ( const listener of listeners ) { - page.addListener( 'dialog', listener ); - } } ); - it( 'should disable all tips if X button is clicked and dialog is confirmed', async () => { + it( 'should disable all tips if X button is clicked and confirmation is confirmed', async () => { // Open up the inserter. await openGlobalBlockInserter(); - // Dismiss the tip. (The confirm() dialog will automatically be accepted.) + // Click the tip's X button. await page.click( '.block-editor-inserter__tip button[aria-label="Dismiss this notice"]' ); + // Accept the confirmation modal. + const [ disableTipsButton ] = await page.$x( '//*[contains(@class, "nux-hide-tips-confirmation")]//button[text()="Disable Tips"]' ); + await disableTipsButton.click(); + // The tip should be gone. const inserterTip = await page.$( '.block-editor-inserter__tip' ); expect( inserterTip ).toBeNull(); diff --git a/packages/nux/src/components/inline-tip/index.js b/packages/nux/src/components/inline-tip/index.js index a1bec5742b21d3..de1e8f4b05b9fe 100644 --- a/packages/nux/src/components/inline-tip/index.js +++ b/packages/nux/src/components/inline-tip/index.js @@ -1,40 +1,80 @@ +/** + * External dependencies + */ +import classnames from 'classnames'; + /** * WordPress dependencies */ -import { Notice } from '@wordpress/components'; +import { useState } from '@wordpress/element'; +import { Notice, Modal, Button } from '@wordpress/components'; import { compose } from '@wordpress/compose'; import { withSelect, withDispatch } from '@wordpress/data'; import { __ } from '@wordpress/i18n'; -export function InlineTip( { isVisible, className, onRemove, children } ) { - if ( ! isVisible ) { - return null; - } +export function InlineTip( { + children, + className, + isTipVisible, + onDisableTips, + onDismissTip, +} ) { + const [ isConfirmationVisible, setIsConfirmationVisible ] = useState( false ); + + const openConfirmation = () => setIsConfirmationVisible( true ); + const closeConfirmation = () => setIsConfirmationVisible( false ); + + const dismissTip = () => { + onDismissTip(); + closeConfirmation(); + }; + + const disableTips = () => { + onDisableTips(); + closeConfirmation(); + }; return ( - - { children } - + <> + { isTipVisible && ( + + { children } + + ) } + + { isConfirmationVisible && ( + + { __( 'Would you like to disable tips like these in the future?' ) } +
+ + +
+ + ) } + ); } export default compose( withSelect( ( select, { tipId } ) => ( { - isVisible: select( 'core/nux' ).isTipVisible( tipId ), + isTipVisible: select( 'core/nux' ).isTipVisible( tipId ), } ) ), withDispatch( ( dispatch, { tipId } ) => { const { disableTips, dismissTip } = dispatch( 'core/nux' ); - return { - onRemove() { - // Disable reason: We don't yet have a component. One day! - // eslint-disable-next-line no-alert - if ( window.confirm( __( 'Would you like to disable tips like these in the future? ' ) ) ) { - disableTips(); - } else { - dismissTip( tipId ); - } - }, + onDismissTip: () => dismissTip( tipId ), + onDisableTips: disableTips, }; } ) )( InlineTip ); diff --git a/packages/nux/src/components/inline-tip/style.scss b/packages/nux/src/components/inline-tip/style.scss new file mode 100644 index 00000000000000..6911c53ce47fef --- /dev/null +++ b/packages/nux/src/components/inline-tip/style.scss @@ -0,0 +1,13 @@ +.nux-hide-tips-confirmation .components-modal__header { + border-bottom: none; + margin-bottom: 0; +} + +.nux-hide-tips-confirmation__buttons { + margin-top: 2rem; + text-align: right; + + .components-button { + margin-left: 10px; + } +} diff --git a/packages/nux/src/components/inline-tip/test/__snapshots__/index.js.snap b/packages/nux/src/components/inline-tip/test/__snapshots__/index.js.snap index fb596ab9500806..3472d2dcd88776 100644 --- a/packages/nux/src/components/inline-tip/test/__snapshots__/index.js.snap +++ b/packages/nux/src/components/inline-tip/test/__snapshots__/index.js.snap @@ -1,34 +1,12 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`InlineTip should render correctly 1`] = ` -
-
+ It looks like you’re writing a letter. Would you like help? -
- -
+ + `; diff --git a/packages/nux/src/components/inline-tip/test/index.js b/packages/nux/src/components/inline-tip/test/index.js index ef66a95b848062..8f9f5c3c4c7279 100644 --- a/packages/nux/src/components/inline-tip/test/index.js +++ b/packages/nux/src/components/inline-tip/test/index.js @@ -1,12 +1,7 @@ /** * External dependencies */ -import TestRenderer from 'react-test-renderer'; - -/** - * WordPress dependencies - */ -import { Notice } from '@wordpress/components'; +import { shallow, mount } from 'enzyme'; /** * Internal dependencies @@ -15,31 +10,44 @@ import { InlineTip } from '../'; describe( 'InlineTip', () => { it( 'should not render anything if invisible', () => { - const renderer = TestRenderer.create( - + const wrapper = shallow( + It looks like you’re writing a letter. Would you like help? ); - expect( renderer.root.children ).toHaveLength( 0 ); + expect( wrapper.children() ).toHaveLength( 0 ); } ); it( 'should render correctly', () => { - const renderer = TestRenderer.create( - + const wrapper = shallow( + + It looks like you’re writing a letter. Would you like help? + + ); + expect( wrapper ).toMatchSnapshot(); + } ); + + it( 'calls `onDismissTip` when the tip and confirmation are dismissed', () => { + const onDismissTip = jest.fn(); + const wrapper = mount( + It looks like you’re writing a letter. Would you like help? ); - expect( renderer ).toMatchSnapshot(); + wrapper.find( 'button[aria-label="Dismiss this notice"]' ).simulate( 'click' ); + wrapper.find( 'button' ).find( { children: 'No' } ).simulate( 'click' ); + expect( onDismissTip ).toHaveBeenCalled(); } ); - it( 'calls `onRemove` when the rendered notice is dismissed', () => { - const onRemove = jest.fn(); - const renderer = TestRenderer.create( - + it( 'calls `onDisableTips` when the tip is dismissed and the confirmation is accepted', () => { + const onDisableTips = jest.fn(); + const wrapper = mount( + It looks like you’re writing a letter. Would you like help? ); - renderer.root.findByType( Notice ).props.onRemove(); - expect( onRemove ).toHaveBeenCalled(); + wrapper.find( 'button[aria-label="Dismiss this notice"]' ).simulate( 'click' ); + wrapper.find( 'button' ).find( { children: 'Disable Tips' } ).simulate( 'click' ); + expect( onDisableTips ).toHaveBeenCalled(); } ); } ); diff --git a/packages/nux/src/style.scss b/packages/nux/src/style.scss index 0df73ff851e9f9..57e39958ceb7a2 100644 --- a/packages/nux/src/style.scss +++ b/packages/nux/src/style.scss @@ -1 +1,2 @@ @import "./components/dot-tip/style.scss"; +@import "./components/inline-tip/style.scss";