Skip to content

Commit

Permalink
Revert "NUX: Prevent duplicate DotTips from appearing (#8642)"
Browse files Browse the repository at this point in the history
This reverts commit 09dc71c.
  • Loading branch information
noisysocks committed Aug 22, 2018
1 parent 0e8c5e6 commit 1bb5ced
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 334 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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>
`;

Expand All @@ -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>
`;

Expand All @@ -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>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -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>
`;
4 changes: 1 addition & 3 deletions packages/nux/src/components/dot-tip/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
104 changes: 42 additions & 62 deletions packages/nux/src/components/dot-tip/index.js
Original file line number Diff line number Diff line change
@@ -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 );
},
Expand Down
8 changes: 4 additions & 4 deletions packages/nux/src/components/dot-tip/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>
);
Expand All @@ -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>
);
Expand All @@ -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>
);
Expand All @@ -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>
);
Expand Down
37 changes: 0 additions & 37 deletions packages/nux/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
37 changes: 1 addition & 36 deletions packages/nux/src/store/reducer.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { uniq, without } from 'lodash';

/**
* WordPress dependencies
*/
Expand All @@ -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.
*
Expand Down Expand Up @@ -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 } );
31 changes: 10 additions & 21 deletions packages/nux/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading

0 comments on commit 1bb5ced

Please sign in to comment.