NUX: Prevent duplicate DotTips from appearing (#8642)

When there are multiple instances of `DotTip` that reference the same
tip identifier, only show the first instance that was mounted.

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
   <WithSelect(WithDispatch(Inserter))
     position="top right"
   >
-    <WithSelect(WithDispatch(DotTip))
+    <WithInstanceId(WithSelect(WithDispatch(DotTip)))
       id="core/editor.inserter"
     >
       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!
-    </WithSelect(WithDispatch(DotTip))>
+    </WithInstanceId(WithSelect(WithDispatch(DotTip)))>
   </WithSelect(WithDispatch(Inserter))>
 </div>
 `;
@@ -72,11 +72,11 @@ exports[`DefaultBlockAppender should match snapshot 1`] = `
   <WithSelect(WithDispatch(Inserter))
     position="top right"
   >
-    <WithSelect(WithDispatch(DotTip))
+    <WithInstanceId(WithSelect(WithDispatch(DotTip)))
       id="core/editor.inserter"
     >
       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!
-    </WithSelect(WithDispatch(DotTip))>
+    </WithInstanceId(WithSelect(WithDispatch(DotTip)))>
   </WithSelect(WithDispatch(Inserter))>
 </div>
 `;
@@ -102,11 +102,11 @@ exports[`DefaultBlockAppender should optionally show without prompt 1`] = `
   <WithSelect(WithDispatch(Inserter))
     position="top right"
   >
-    <WithSelect(WithDispatch(DotTip))
+    <WithInstanceId(WithSelect(WithDispatch(DotTip)))
       id="core/editor.inserter"
     >
       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!
-    </WithSelect(WithDispatch(DotTip))>
+    </WithInstanceId(WithSelect(WithDispatch(DotTip)))>
   </WithSelect(WithDispatch(Inserter))>
 </div>
 `;

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
-  <WithSelect(WithDispatch(DotTip))
+  <WithInstanceId(WithSelect(WithDispatch(DotTip)))
     id="core/editor.preview"
   >
     Click ‘Preview’ to load a preview of this page, so you can make sure you’re happy with your blocks.
-  </WithSelect(WithDispatch(DotTip))>
+  </WithInstanceId(WithSelect(WithDispatch(DotTip)))>
 </Button>
 `;

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

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 (
-		<Popover
-			className="nux-dot-tip"
-			position="middle right"
-			noArrow
-			focusOnMount="container"
-			role="dialog"
-			aria-label={ __( 'Gutenberg tips' ) }
-			onClick={ ( event ) => {
-				// 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();
-			} }
-		>
-			<p>{ children }</p>
-			<p>
-				<Button isLink onClick={ onDismiss }>
-					{ hasNextTip ? __( 'See next tip' ) : __( 'Got it' ) }
-				</Button>
-			</p>
-			<IconButton
-				className="nux-dot-tip__disable"
-				icon="no-alt"
-				label={ __( 'Disable tips' ) }
-				onClick={ onDisable }
-			/>
-		</Popover>
-	);
+	componentWillUnmount() {
+		this.props.onUnregister();
+	}
+
+	render() {
+		const { children, isVisible, hasNextTip, onDismiss, onDisable } = this.props;
+
+		if ( ! isVisible ) {
+			return null;
+		}
+
+		return (
+			<Popover
+				className="nux-dot-tip"
+				position="middle right"
+				noArrow
+				focusOnMount="container"
+				role="dialog"
+				aria-label={ __( 'Gutenberg tips' ) }
+				onClick={ ( event ) => {
+					// 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();
+				} }
+			>
+				<p>{ children }</p>
+				<p>
+					<Button isLink onClick={ onDismiss }>
+						{ hasNextTip ? __( 'See next tip' ) : __( 'Got it' ) }
+					</Button>
+				</p>
+				<IconButton
+					className="nux-dot-tip__disable"
+					icon="no-alt"
+					label={ __( 'Disable tips' ) }
+					onClick={ onDisable }
+				/>
+			</Popover>
+		);
+	}
 }
 
 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 );
 			},

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(
-			<DotTip>
+			<DotTip onRegister={ noop } onUnregister={ noop }>
 				It looks like you’re writing a letter. Would you like help?
 			</DotTip>
 		);
@@ -23,7 +23,7 @@ describe( 'DotTip', () => {
 
 	it( 'should render correctly', () => {
 		const wrapper = shallow(
-			<DotTip isVisible setTimeout={ noop }>
+			<DotTip isVisible onRegister={ noop } onUnregister={ noop }>
 				It looks like you’re writing a letter. Would you like help?
 			</DotTip>
 		);
@@ -33,7 +33,7 @@ describe( 'DotTip', () => {
 	it( 'should call onDismiss when the dismiss button is clicked', () => {
 		const onDismiss = jest.fn();
 		const wrapper = shallow(
-			<DotTip isVisible onDismiss={ onDismiss } setTimeout={ noop }>
+			<DotTip isVisible onDismiss={ onDismiss } onRegister={ noop } onUnregister={ noop }>
 				It looks like you’re writing a letter. Would you like help?
 			</DotTip>
 		);
@@ -44,7 +44,7 @@ describe( 'DotTip', () => {
 	it( 'should call onDisable when the X button is clicked', () => {
 		const onDisable = jest.fn();
 		const wrapper = shallow(
-			<DotTip isVisible onDisable={ onDisable } setTimeout={ noop }>
+			<DotTip isVisible onDisable={ onDisable } onRegister={ noop } onUnregister={ noop }>
 				It looks like you’re writing a letter. Would you like help?
 			</DotTip>
 		);

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.

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 } );

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;
 }