diff --git a/packages/editor/src/components/default-block-appender/test/__snapshots__/index.js.snap b/packages/editor/src/components/default-block-appender/test/__snapshots__/index.js.snap index 13ff5d0983c1cf..86ff2fe9f55ef7 100644 --- a/packages/editor/src/components/default-block-appender/test/__snapshots__/index.js.snap +++ b/packages/editor/src/components/default-block-appender/test/__snapshots__/index.js.snap @@ -42,11 +42,11 @@ exports[`DefaultBlockAppender should append a default block when input focused 1 - Welcome to the wonderful world of blocks! Click the “+” (“Add block”) button to add a new block. There are blocks available for all kind of content: you can insert text, headings, images, lists, and lots more! - + `; @@ -72,11 +72,11 @@ exports[`DefaultBlockAppender should match snapshot 1`] = ` - Welcome to the wonderful world of blocks! Click the “+” (“Add block”) button to add a new block. There are blocks available for all kind of content: you can insert text, headings, images, lists, and lots more! - + `; @@ -102,11 +102,11 @@ exports[`DefaultBlockAppender should optionally show without prompt 1`] = ` - Welcome to the wonderful world of blocks! Click the “+” (“Add block”) button to add a new block. There are blocks available for all kind of content: you can insert text, headings, images, lists, and lots more! - + `; diff --git a/packages/editor/src/components/post-preview-button/test/__snapshots__/index.js.snap b/packages/editor/src/components/post-preview-button/test/__snapshots__/index.js.snap index fd1cc57238af17..2b142e65c140b5 100644 --- a/packages/editor/src/components/post-preview-button/test/__snapshots__/index.js.snap +++ b/packages/editor/src/components/post-preview-button/test/__snapshots__/index.js.snap @@ -8,10 +8,10 @@ exports[`PostPreviewButton render() should match the snapshot 1`] = ` onClick={[Function]} > Preview - Click ‘Preview’ to load a preview of this page, so you can make sure you’re happy with your blocks. - + `; diff --git a/packages/nux/src/components/dot-tip/README.md b/packages/nux/src/components/dot-tip/README.md index caf164760ffeca..4fb33bb9ddae39 100644 --- a/packages/nux/src/components/dot-tip/README.md +++ b/packages/nux/src/components/dot-tip/README.md @@ -3,6 +3,8 @@ DotTip `DotTip` is a React component that renders a single _tip_ on the screen. The tip will point to the React element that `DotTip` is nested within. Each tip is uniquely identified by a string passed to `id`. +When there are multiple instances of `DotTip` that reference the same tip identifier, only the first instance to have been mounted will be visible. + ## Usage ```jsx @@ -21,7 +23,7 @@ The component accepts the following props: ### id -A string that uniquely identifies the tip. Identifiers should be prefixed with the name of the plugin, followed by a `/`. For example, `acme/add-to-cart`. +A string that uniquely identifies the tip. Identifiers should be prefixed with the name of the plugin, followed by a `/`. For example, `acme/add-to-cart`. Changing the identifier after the component has mounted is not supported. - Type: `string` - Required: Yes diff --git a/packages/nux/src/components/dot-tip/index.js b/packages/nux/src/components/dot-tip/index.js index b41c794e7193bf..80e33285906fbf 100644 --- a/packages/nux/src/components/dot-tip/index.js +++ b/packages/nux/src/components/dot-tip/index.js @@ -1,64 +1,84 @@ /** * WordPress dependencies */ -import { compose } from '@wordpress/compose'; +import { Component } from '@wordpress/element'; +import { compose, withInstanceId } from '@wordpress/compose'; import { Popover, Button, IconButton } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { withSelect, withDispatch } from '@wordpress/data'; -export function DotTip( { - children, - isVisible, - hasNextTip, - onDismiss, - onDisable, -} ) { - if ( ! isVisible ) { - return null; +export class DotTip extends Component { + componentDidMount() { + this.props.onRegister(); } - return ( - { - // Tips are often nested within buttons. We stop propagation so that clicking - // on a tip doesn't result in the button being clicked. - event.stopPropagation(); - } } - > -

{ children }

-

- -

- - - ); + componentWillUnmount() { + this.props.onUnregister(); + } + + render() { + const { children, isVisible, hasNextTip, onDismiss, onDisable } = this.props; + + if ( ! isVisible ) { + return null; + } + + return ( + { + // Tips are often nested within buttons. We stop propagation so that clicking + // on a tip doesn't result in the button being clicked. + event.stopPropagation(); + } } + > +

{ children }

+

+ +

+ + + ); + } } export default compose( - withSelect( ( select, { id } ) => { + withInstanceId, + withSelect( ( select, { id, instanceId } ) => { const { isTipVisible, getAssociatedGuide } = select( 'core/nux' ); const associatedGuide = getAssociatedGuide( id ); return { - isVisible: isTipVisible( id ), + isVisible: isTipVisible( id, instanceId ), hasNextTip: !! ( associatedGuide && associatedGuide.nextTipId ), }; } ), - withDispatch( ( dispatch, { id } ) => { - const { dismissTip, disableTips } = dispatch( 'core/nux' ); + withDispatch( ( dispatch, { id, instanceId } ) => { + const { + registerTipInstance, + unregisterTipInstance, + dismissTip, + disableTips, + } = dispatch( 'core/nux' ); + return { + onRegister() { + registerTipInstance( id, instanceId ); + }, + onUnregister() { + unregisterTipInstance( id, instanceId ); + }, onDismiss() { dismissTip( id ); }, diff --git a/packages/nux/src/components/dot-tip/test/index.js b/packages/nux/src/components/dot-tip/test/index.js index aa69872d5e6e80..884c9422ecb77c 100644 --- a/packages/nux/src/components/dot-tip/test/index.js +++ b/packages/nux/src/components/dot-tip/test/index.js @@ -14,7 +14,7 @@ jest.mock( '../../../../../components/src/button' ); describe( 'DotTip', () => { it( 'should not render anything if invisible', () => { const wrapper = shallow( - + It looks like you’re writing a letter. Would you like help? ); @@ -23,7 +23,7 @@ describe( 'DotTip', () => { it( 'should render correctly', () => { const wrapper = shallow( - + It looks like you’re writing a letter. Would you like help? ); @@ -33,7 +33,7 @@ describe( 'DotTip', () => { it( 'should call onDismiss when the dismiss button is clicked', () => { const onDismiss = jest.fn(); const wrapper = shallow( - + It looks like you’re writing a letter. Would you like help? ); @@ -44,7 +44,7 @@ describe( 'DotTip', () => { it( 'should call onDisable when the X button is clicked', () => { const onDisable = jest.fn(); const wrapper = shallow( - + It looks like you’re writing a letter. Would you like help? ); diff --git a/packages/nux/src/store/actions.js b/packages/nux/src/store/actions.js index ad8adb79c5530d..3ee9c8e142a891 100644 --- a/packages/nux/src/store/actions.js +++ b/packages/nux/src/store/actions.js @@ -13,6 +13,43 @@ export function triggerGuide( tipIds ) { }; } +/** + * Returns an action object that, when dispathed, associates an instance of the + * DotTip component with a tip. This is usually done when the component mounts. + * Tracking this lets us only show one DotTip at a time per tip. + * + * @param {string} tipId The tip to associate this instance with. + * @param {number} instanceId A number which uniquely identifies the instance. + * + * @return {Object} Action object. + */ +export function registerTipInstance( tipId, instanceId ) { + return { + type: 'REGISTER_TIP_INSTANCE', + tipId, + instanceId, + }; +} + +/** + * Returns an action object that, when dispatched, removes the association + * between a DotTip component instance and a tip. This is usually done when the + * component unmounts. Tracking this lets us only show one DotTip at a time per + * tip. + * + * @param {string} tipId The tip to disassociate this instance with. + * @param {number} instanceId A number which uniquely identifies the instance. + * + * @return {Object} Action object. + */ +export function unregisterTipInstance( tipId, instanceId ) { + return { + type: 'UNREGISTER_TIP_INSTANCE', + tipId, + instanceId, + }; +} + /** * Returns an action object that, when dispatched, dismisses the given tip. A * dismissed tip will not show again. diff --git a/packages/nux/src/store/reducer.js b/packages/nux/src/store/reducer.js index 5ebf8ea394e70b..2383fee47b4efe 100644 --- a/packages/nux/src/store/reducer.js +++ b/packages/nux/src/store/reducer.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { union, without } from 'lodash'; + /** * WordPress dependencies */ @@ -24,6 +29,34 @@ export function guides( state = [], action ) { return state; } +/** + * Reducer that maps each tip to an array of DotTip component instance IDs that are + * displaying that tip. Tracking this allows us to only show one DotTip at a + * time per tip. + * + * @param {Object} state Current state. + * @param {Object} action Dispatched action. + * + * @return {Object} Updated state. + */ +export function tipInstanceIds( state = {}, action ) { + switch ( action.type ) { + case 'REGISTER_TIP_INSTANCE': + return { + ...state, + [ action.tipId ]: union( state[ action.tipId ] || [], [ action.instanceId ] ), + }; + + case 'UNREGISTER_TIP_INSTANCE': + return { + ...state, + [ action.tipId ]: without( state[ action.tipId ], action.instanceId ), + }; + } + + return state; +} + /** * Reducer that tracks whether or not tips are globally enabled. * @@ -70,4 +103,4 @@ export function dismissedTips( state = {}, action ) { const preferences = combineReducers( { areTipsEnabled, dismissedTips } ); -export default combineReducers( { guides, preferences } ); +export default combineReducers( { guides, tipInstanceIds, preferences } ); diff --git a/packages/nux/src/store/selectors.js b/packages/nux/src/store/selectors.js index 61f0193d90f46c..149cd30dfb0a3e 100644 --- a/packages/nux/src/store/selectors.js +++ b/packages/nux/src/store/selectors.js @@ -41,29 +41,40 @@ export const getAssociatedGuide = createSelector( ); /** - * Determines whether or not the given tip is showing. Tips are hidden if they - * are disabled, have been dismissed, or are not the current tip in any - * guide that they have been added to. + * Determines whether the given tip or DotTip instance should be visible. Checks: * - * @param {Object} state Global application state. - * @param {string} id The tip to query. + * - That all tips are enabled. + * - That the given tip has not been dismissed. + * - If the given tip is part of a guide, that the given tip is the current tip in the guide. + * - If instanceId is provided, that this is the first DotTip instance for the given tip. + * + * @param {Object} state Global application state. + * @param {string} tipId The tip to query. + * @param {?number} instanceId A number which uniquely identifies the DotTip instance. * - * @return {boolean} Whether or not the given tip is showing. + * @return {boolean} Whether the given tip or Dottip instance should be shown. */ -export function isTipVisible( state, id ) { +export function isTipVisible( state, tipId, instanceId ) { if ( ! state.preferences.areTipsEnabled ) { return false; } - if ( state.preferences.dismissedTips[ id ] ) { + if ( state.preferences.dismissedTips[ tipId ] ) { return false; } - const associatedGuide = getAssociatedGuide( state, id ); - if ( associatedGuide && associatedGuide.currentTipId !== id ) { + const associatedGuide = getAssociatedGuide( state, tipId ); + if ( associatedGuide && associatedGuide.currentTipId !== tipId ) { return false; } + if ( instanceId ) { + const [ firstInstanceId ] = state.tipInstanceIds[ tipId ] || []; + if ( instanceId !== firstInstanceId ) { + return false; + } + } + return true; } diff --git a/packages/nux/src/store/test/actions.js b/packages/nux/src/store/test/actions.js index 4e22afe03c8b82..7bdf580fe9eb83 100644 --- a/packages/nux/src/store/test/actions.js +++ b/packages/nux/src/store/test/actions.js @@ -1,7 +1,14 @@ /** * Internal dependencies */ -import { triggerGuide, dismissTip, disableTips, enableTips } from '../actions'; +import { + triggerGuide, + registerTipInstance, + unregisterTipInstance, + dismissTip, + disableTips, + enableTips, +} from '../actions'; describe( 'actions', () => { describe( 'triggerGuide', () => { @@ -13,6 +20,26 @@ describe( 'actions', () => { } ); } ); + describe( 'registerTipInstance', () => { + it( 'should return a REGISTER_TIP_INSTANCE action', () => { + expect( registerTipInstance( 'test/tip-1', 123 ) ).toEqual( { + type: 'REGISTER_TIP_INSTANCE', + tipId: 'test/tip-1', + instanceId: 123, + } ); + } ); + } ); + + describe( 'unregisterTipInstance', () => { + it( 'should return an UNREGISTER_TIP_INSTANCE action', () => { + expect( unregisterTipInstance( 'test/tip-1', 123 ) ).toEqual( { + type: 'UNREGISTER_TIP_INSTANCE', + tipId: 'test/tip-1', + instanceId: 123, + } ); + } ); + } ); + describe( 'dismissTip', () => { it( 'should return an DISMISS_TIP action', () => { expect( dismissTip( 'test/tip' ) ).toEqual( { diff --git a/packages/nux/src/store/test/reducer.js b/packages/nux/src/store/test/reducer.js index d2982a87d70501..81197e83b0942d 100644 --- a/packages/nux/src/store/test/reducer.js +++ b/packages/nux/src/store/test/reducer.js @@ -1,7 +1,12 @@ +/** + * External dependencies + */ +import deepFreeze from 'deep-freeze'; + /** * Internal dependencies */ -import { guides, areTipsEnabled, dismissedTips } from '../reducer'; +import { guides, tipInstanceIds, areTipsEnabled, dismissedTips } from '../reducer'; describe( 'reducer', () => { describe( 'guides', () => { @@ -10,7 +15,8 @@ describe( 'reducer', () => { } ); it( 'should add a guide when it is triggered', () => { - const state = guides( [], { + const original = deepFreeze( [] ); + const state = guides( original, { type: 'TRIGGER_GUIDE', tipIds: [ 'test/tip-1', 'test/tip-2' ], } ); @@ -20,6 +26,66 @@ describe( 'reducer', () => { } ); } ); + describe( 'tipInstanceIds', () => { + it( 'should start out empty', () => { + expect( tipInstanceIds( undefined, {} ) ).toEqual( {} ); + } ); + + it( 'should register an initial tip instance', () => { + const original = deepFreeze( {} ); + const state = tipInstanceIds( original, { + type: 'REGISTER_TIP_INSTANCE', + tipId: 'test/tip-1', + instanceId: 123, + } ); + expect( state ).toEqual( { + 'test/tip-1': [ 123 ], + } ); + } ); + + it( 'should register an second tip instance', () => { + const original = deepFreeze( { + 'test/tip-1': [ 123 ], + } ); + const state = tipInstanceIds( original, { + type: 'REGISTER_TIP_INSTANCE', + tipId: 'test/tip-1', + instanceId: 456, + } ); + expect( state ).toEqual( { + 'test/tip-1': [ 123, 456 ], + } ); + } ); + + it( 'should not register duplicate tip instances', () => { + const original = deepFreeze( { + 'test/tip-1': [ 123 ], + } ); + const state = tipInstanceIds( original, { + type: 'REGISTER_TIP_INSTANCE', + tipId: 'test/tip-1', + instanceId: 123, + } ); + expect( state ).toEqual( { + 'test/tip-1': [ 123 ], + } ); + } ); + + it( 'should unregister a tip instance', () => { + const original = deepFreeze( { + 'test/tip-1': [ 123, 456 ], + } ); + const state = tipInstanceIds( original, { + type: 'UNREGISTER_TIP_INSTANCE', + tipId: 'test/tip-1', + instanceId: 123, + } ); + expect( state ).toEqual( { + 'test/tip-1': [ 456 ], + } ); + } ); + } ); + describe( 'areTipsEnabled', () => { it( 'should default to true', () => { expect( areTipsEnabled( undefined, {} ) ).toBe( true ); @@ -46,7 +112,8 @@ describe( 'reducer', () => { } ); it( 'should mark tips as dismissed', () => { - const state = dismissedTips( {}, { + const original = deepFreeze( {} ); + const state = dismissedTips( original, { type: 'DISMISS_TIP', id: 'test/tip', } ); @@ -56,10 +123,10 @@ describe( 'reducer', () => { } ); it( 'should reset if tips are enabled', () => { - const initialState = { + const original = deepFreeze( { 'test/tip': true, - }; - const state = dismissedTips( initialState, { + } ); + const state = dismissedTips( original, { type: 'ENABLE_TIPS', } ); expect( state ).toEqual( {} ); diff --git a/packages/nux/src/store/test/selectors.js b/packages/nux/src/store/test/selectors.js index ed63a7fa828de5..f807957abe46d5 100644 --- a/packages/nux/src/store/test/selectors.js +++ b/packages/nux/src/store/test/selectors.js @@ -11,6 +11,7 @@ describe( 'selectors', () => { [ 'test/tip-a', 'test/tip-b', 'test/tip-c' ], [ 'test/tip-α', 'test/tip-β', 'test/tip-γ' ], ], + tipInstanceIds: {}, preferences: { dismissedTips: { 'test/tip-1': true, @@ -56,6 +57,7 @@ describe( 'selectors', () => { it( 'should return true by default', () => { const state = { guides: [], + tipInstanceIds: {}, preferences: { areTipsEnabled: true, dismissedTips: {}, @@ -67,6 +69,7 @@ describe( 'selectors', () => { it( 'should return false if tips are disabled', () => { const state = { guides: [], + tipInstanceIds: {}, preferences: { areTipsEnabled: false, dismissedTips: {}, @@ -78,6 +81,7 @@ describe( 'selectors', () => { it( 'should return false if the tip is dismissed', () => { const state = { guides: [], + tipInstanceIds: {}, preferences: { areTipsEnabled: true, dismissedTips: { @@ -93,6 +97,7 @@ describe( 'selectors', () => { guides: [ [ 'test/tip-1', 'test/tip-2', 'test/tip-3' ], ], + tipInstanceIds: {}, preferences: { areTipsEnabled: true, dismissedTips: {}, @@ -100,12 +105,53 @@ describe( 'selectors', () => { }; expect( isTipVisible( state, 'test/tip-2' ) ).toBe( false ); } ); + + it( 'should return true if an instanceId that was registered first is provided', () => { + const state = { + guides: [], + tipInstanceIds: { + 'test/tip': [ 123, 456 ], + }, + preferences: { + areTipsEnabled: true, + dismissedTips: {}, + }, + }; + expect( isTipVisible( state, 'test/tip', 123 ) ).toBe( true ); + } ); + + it( 'should return false if an instanceId that was NOT registered is provided', () => { + const state = { + guides: [], + tipInstanceIds: {}, + preferences: { + areTipsEnabled: true, + dismissedTips: {}, + }, + }; + expect( isTipVisible( state, 'test/tip', 123 ) ).toBe( false ); + } ); + + it( 'should return false if an instanceId that was NOT registered first is provided', () => { + const state = { + guides: [], + tipInstanceIds: { + 'test/tip': [ 123, 456 ], + }, + preferences: { + areTipsEnabled: true, + dismissedTips: {}, + }, + }; + expect( isTipVisible( state, 'test/tip', 456 ) ).toBe( false ); + } ); } ); describe( 'areTipsEnabled', () => { it( 'should return true if tips are enabled', () => { const state = { guides: [], + tipInstanceIds: {}, preferences: { areTipsEnabled: true, dismissedTips: {}, @@ -117,6 +163,7 @@ describe( 'selectors', () => { it( 'should return false if tips are disabled', () => { const state = { guides: [], + tipInstanceIds: {}, preferences: { areTipsEnabled: false, dismissedTips: {},