Revert "NUX: Prevent duplicate DotTips from appearing (#8642)"

This reverts commit 09dc71c.
WordPress · Aug 22, 2018 · 1bb5ced · 1bb5ced
1 parent 0e8c5e6
commit 1bb5ced
Show file tree

Hide file tree

Showing 11 changed files with 70 additions and 334 deletions.
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
@@ -25,13 +25,7 @@ exports[`DefaultBlockAppender should append a default block when input focused 1
   <WithSelect(WithDispatch(InserterWithShortcuts)) />
   <WithSelect(WithDispatch(Inserter))
     position="top right"
-  >
-    <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!
-    </WithInstanceId(WithSelect(WithDispatch(DotTip)))>
-  </WithSelect(WithDispatch(Inserter))>
+  />
 </div>
 `;
 
@@ -53,13 +47,7 @@ exports[`DefaultBlockAppender should match snapshot 1`] = `
   <WithSelect(WithDispatch(InserterWithShortcuts)) />
   <WithSelect(WithDispatch(Inserter))
     position="top right"
-  >
-    <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!
-    </WithInstanceId(WithSelect(WithDispatch(DotTip)))>
-  </WithSelect(WithDispatch(Inserter))>
+  />
 </div>
 `;
 
@@ -81,12 +69,6 @@ exports[`DefaultBlockAppender should optionally show without prompt 1`] = `
   <WithSelect(WithDispatch(InserterWithShortcuts)) />
   <WithSelect(WithDispatch(Inserter))
     position="top right"
-  >
-    <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!
-    </WithInstanceId(WithSelect(WithDispatch(DotTip)))>
-  </WithSelect(WithDispatch(Inserter))>
+  />
 </div>
 `;
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
@@ -8,10 +8,10 @@ exports[`PostPreviewButton render() should match the snapshot 1`] = `
   onClick={[Function]}
 >
   Preview
-  <WithInstanceId(WithSelect(WithDispatch(DotTip)))
+  <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.
-  </WithInstanceId(WithSelect(WithDispatch(DotTip)))>
+  </WithSelect(WithDispatch(DotTip))>
 </Button>
 `;
diff --git a/packages/nux/src/components/dot-tip/README.md b/packages/nux/src/components/dot-tip/README.md
@@ -3,8 +3,6 @@ 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
@@ -23,7 +21,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`. Changing the identifier after the component has mounted is not supported.
+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`.
 
 - Type: `string`
 - Required: Yes

diff --git a/packages/nux/src/components/dot-tip/index.js b/packages/nux/src/components/dot-tip/index.js
@@ -1,84 +1,64 @@
 /**
  * WordPress dependencies
  */
-import { Component } from '@wordpress/element';
-import { compose, withInstanceId } from '@wordpress/compose';
+import { compose } from '@wordpress/compose';
 import { Popover, Button, IconButton } from '@wordpress/components';
 import { __ } from '@wordpress/i18n';
 import { withSelect, withDispatch } from '@wordpress/data';
 
-export class DotTip extends Component {
-	componentDidMount() {
-		this.props.onRegister();
+export function DotTip( {
+	children,
+	isVisible,
+	hasNextTip,
+	onDismiss,
+	onDisable,
+} ) {
+	if ( ! isVisible ) {
+		return null;
 	}
 
-	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>
-		);
-	}
+	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(
-	withInstanceId,
-	withSelect( ( select, { id, instanceId } ) => {
+	withSelect( ( select, { id } ) => {
 		const { isTipVisible, getAssociatedGuide } = select( 'core/nux' );
 		const associatedGuide = getAssociatedGuide( id );
 		return {
-			isVisible: isTipVisible( id, instanceId ),
+			isVisible: isTipVisible( id ),
 			hasNextTip: !! ( associatedGuide && associatedGuide.nextTipId ),
 		};
 	} ),
-	withDispatch( ( dispatch, { id, instanceId } ) => {
-		const {
-			registerTipInstance,
-			unregisterTipInstance,
-			dismissTip,
-			disableTips,
-		} = dispatch( 'core/nux' );
-
+	withDispatch( ( dispatch, { id } ) => {
+		const { 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
@@ -14,7 +14,7 @@ jest.mock( '../../../../../components/src/button' );
 describe( 'DotTip', () => {
 	it( 'should not render anything if invisible', () => {
 		const wrapper = shallow(
-			<DotTip onRegister={ noop } onUnregister={ noop }>
+			<DotTip>
 				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 onRegister={ noop } onUnregister={ noop }>
+			<DotTip isVisible setTimeout={ 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 } onRegister={ noop } onUnregister={ noop }>
+			<DotTip isVisible onDismiss={ onDismiss } setTimeout={ 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 } onRegister={ noop } onUnregister={ noop }>
+			<DotTip isVisible onDisable={ onDisable } setTimeout={ noop }>
 				It looks like you’re writing a letter. Would you like help?
 			</DotTip>
 		);

diff --git a/packages/nux/src/store/actions.js b/packages/nux/src/store/actions.js
@@ -13,43 +13,6 @@ export function triggerGuide( tipIds ) {
 	};
 }
 
-/**
- * Returns an action object that, when dispatched, 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
@@ -1,8 +1,3 @@
-/**
- * External dependencies
- */
-import { uniq, without } from 'lodash';
-
 /**
  * WordPress dependencies
  */
@@ -29,36 +24,6 @@ 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': {
-			const existingInstanceIds = state[ action.tipId ] || [];
-			return {
-				...state,
-				[ action.tipId ]: uniq( [ ...existingInstanceIds, 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.
  *
@@ -105,4 +70,4 @@ export function dismissedTips( state = {}, action ) {
 
 const preferences = combineReducers( { areTipsEnabled, dismissedTips } );
 
-export default combineReducers( { guides, tipInstanceIds, preferences } );
+export default combineReducers( { guides, preferences } );
diff --git a/packages/nux/src/store/selectors.js b/packages/nux/src/store/selectors.js
@@ -41,40 +41,29 @@ export const getAssociatedGuide = createSelector(
 );
 
 /**
- * Determines whether the given tip or DotTip instance should be visible. Checks:
+ * 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.
  *
- * - 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.
+ * @param {Object} state Global application state.
+ * @param {string} id    The tip to query.
  *
- * @return {boolean} Whether the given tip or Dottip instance should be shown.
+ * @return {boolean} Whether or not the given tip is showing.
  */
-export function isTipVisible( state, tipId, instanceId ) {
+export function isTipVisible( state, id ) {
 	if ( ! state.preferences.areTipsEnabled ) {
 		return false;
 	}
 
-	if ( state.preferences.dismissedTips[ tipId ] ) {
+	if ( state.preferences.dismissedTips[ id ] ) {
 		return false;
 	}
 
-	const associatedGuide = getAssociatedGuide( state, tipId );
-	if ( associatedGuide && associatedGuide.currentTipId !== tipId ) {
+	const associatedGuide = getAssociatedGuide( state, id );
+	if ( associatedGuide && associatedGuide.currentTipId !== id ) {
 		return false;
 	}
 
-	if ( instanceId ) {
-		const [ firstInstanceId ] = state.tipInstanceIds[ tipId ] || [];
-		if ( instanceId !== firstInstanceId ) {
-			return false;
-		}
-	}
-
 	return true;
 }