Skip to content

Commit

Permalink
Framework: Remove react-tap-event-plugin (#7724)
Browse files Browse the repository at this point in the history
facebook/react#436 (comment) indicates that it's no longer needed.

* Remove react-tap-event-plugin from tests
* Accordion: Use onClick instead of onTouchTap
* Accordion Tests: Use Enzyme to simulate click
* SectionNav: Use onClick instead of onTouchTap
* Components: Replace more onTouchTap instances with onClick
* Blocks: Replace more onTouchTap instances with onClick
* DailyPost[Button]: Fix tests
* Accordion Tests: Simplify some more
* docs/react-component-unit-testing.md: Update interaction simulation example
* Site Block: Drop old onClick prop, rename onTouchTap to onClick
 And modify all consumers to no longer rely on the old onClick prop.
  • Loading branch information
ockham authored Jan 16, 2017
1 parent 4dd8223 commit 1ca3a61
Show file tree
Hide file tree
Showing 24 changed files with 64 additions and 91 deletions.
11 changes: 1 addition & 10 deletions client/blocks/comment-button/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,6 @@ const CommentButton = React.createClass( {
};
},

onClick( event ) {
event.preventDefault();
},

onTap() {
this.props.onClick();
},

render() {
let label;
const containerTag = this.props.tagName,
Expand Down Expand Up @@ -70,8 +62,7 @@ const CommentButton = React.createClass( {
return React.createElement(
containerTag, {
className: 'comment-button',
onTouchTap: this.onTap,
onClick: this.onClick
onClick: this.props.onClick
},
<Gridicon icon="comment" size={ this.props.size } className="comment-button__icon" />, labelElement
);
Expand Down
9 changes: 2 additions & 7 deletions client/blocks/daily-post-button/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ function getPingbackAttributes( post ) {
};
}

function preventDefault( event ) {
event.preventDefault();
}

function preloadEditor() {
preload( 'post-editor' );
}
Expand Down Expand Up @@ -107,7 +103,7 @@ class DailyPostButton extends React.Component {
}

toggle = ( event ) => {
preventDefault( event );
event.preventDefault();
if ( ! this.state.showingMenu ) {
recordAction( 'open_daily_post_challenge' );
recordGaEvent( 'Opened Daily Post Challenge' );
Expand Down Expand Up @@ -161,8 +157,7 @@ class DailyPostButton extends React.Component {

return React.createElement( this.props.tagName, {
className: 'daily-post-button',
onTouchTap: this.toggle,
onClick: preventDefault,
onClick: this.toggle,
onTouchStart: preloadEditor,
onMouseEnter: preloadEditor
}, [
Expand Down
4 changes: 2 additions & 2 deletions client/blocks/daily-post-button/test/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,13 @@ describe( 'DailyPostButton', () => {

it( 'rediects to primary site if the user only has one site', () => {
sitesListMock.data = sitesListMock.data.slice( 0, 1 );
dailyPostButton.simulate( 'touchTap', { preventDefault: noop } );
dailyPostButton.simulate( 'click', { preventDefault: noop } );
assert.isTrue( pageSpy.calledWithMatch( /post\/apps.wordpress.com?/ ) );
} );

it( 'shows the site selector if the user has more than one site', ( done ) => {
dailyPostButton.instance().renderSitesPopover = done;
dailyPostButton.simulate( 'touchTap', { preventDefault: noop } );
dailyPostButton.simulate( 'click', { preventDefault: noop } );
} );
} );

Expand Down
2 changes: 1 addition & 1 deletion client/blocks/like-button/button.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ const LikeButton = React.createClass( {
containerTag,
{
className: classNames( containerClasses ),
onTouchTap: this.toggleLiked
onClick: this.toggleLiked
},
<LikeIcons size={ this.props.iconSize } />, labelElement
)
Expand Down
5 changes: 1 addition & 4 deletions client/blocks/site/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ export default React.createClass( {
return {
// onSelect callback
onSelect: noop,
onClick: noop,
// mouse event callbacks
onMouseEnter: noop,
onMouseLeave: noop,
Expand Down Expand Up @@ -48,7 +47,6 @@ export default React.createClass( {
isSelected: React.PropTypes.bool,
isHighlighted: React.PropTypes.bool,
site: React.PropTypes.object.isRequired,
onClick: React.PropTypes.func,
homeLink: React.PropTypes.bool,
showHomeIcon: React.PropTypes.bool
},
Expand Down Expand Up @@ -98,8 +96,7 @@ export default React.createClass( {
? this.translate( 'View this site' )
: this.translate( 'Select this site' )
}
onTouchTap={ this.onSelect }
onClick={ this.props.onClick }
onClick={ this.onSelect }
onMouseEnter={ this.onMouseEnter }
onMouseLeave={ this.onMouseLeave }
aria-label={ this.props.homeLink && site.is_previewable
Expand Down
4 changes: 0 additions & 4 deletions client/boot/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ var React = require( 'react' ),
page = require( 'page' ),
url = require( 'url' ),
qs = require( 'querystring' ),
injectTapEventPlugin = require( 'react-tap-event-plugin' ),
i18n = require( 'i18n-calypso' ),
includes = require( 'lodash/includes' );

Expand Down Expand Up @@ -80,9 +79,6 @@ function init() {
document.documentElement.classList.add( 'notouch' );
}

// Initialize touch
injectTapEventPlugin();

// Add accessible-focus listener
accessibleFocus();
}
Expand Down
2 changes: 1 addition & 1 deletion client/components/accordion/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export default class Accordion extends Component {
return (
<div className={ classes }>
<header className="accordion__header">
<button type="button" onTouchTap={ this.toggleExpanded } className="accordion__toggle">
<button type="button" onClick={ this.toggleExpanded } className="accordion__toggle">
{ icon && <span className="accordion__icon">{ icon }</span> }
<span className="accordion__title">{ title }</span>
{ subtitle && <span className="accordion__subtitle">{ subtitle }</span> }
Expand Down
29 changes: 9 additions & 20 deletions client/components/accordion/test/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,16 @@
* External dependencies
*/
import { expect } from 'chai';
import ReactDom from 'react-dom';
import React from 'react';
import TestUtils from 'react-addons-test-utils';
import { shallow, mount } from 'enzyme';
import createReactTapEventPlugin from 'react-tap-event-plugin';
import { shallow } from 'enzyme';

/**
* Internal dependencies
*/
import useFakeDom from 'test/helpers/use-fake-dom';
import Gridicon from 'components/gridicon';
import Accordion from '../';

describe( 'Accordion', function() {
before( () => {
// Unfortunately, there is no corresponding teardown for this plugin
createReactTapEventPlugin();
} );

it( 'should render as expected with a title and content', function() {
const wrapper = shallow( <Accordion title="Section">Content</Accordion> );

Expand Down Expand Up @@ -50,24 +41,22 @@ describe( 'Accordion', function() {
} );

context( 'events', () => {
useFakeDom();

function simulateTouchTap( wrapper ) {
TestUtils.Simulate.touchTap( ReactDom.findDOMNode( wrapper.find( '.accordion__toggle' ).node ) );
function simulateClick( wrapper ) {
wrapper.find( '.accordion__toggle' ).simulate( 'click' );
}

it( 'should toggle when clicked', function() {
const wrapper = mount( <Accordion title="Section">Content</Accordion> );
const wrapper = shallow( <Accordion title="Section">Content</Accordion> );

simulateTouchTap( wrapper );
simulateClick( wrapper );

expect( wrapper ).to.have.state( 'isExpanded' ).be.true;
} );

it( 'should accept an onToggle function handler to be invoked when toggled', function( done ) {
const wrapper = mount( <Accordion title="Section" onToggle={ finishTest }>Content</Accordion> );
const wrapper = shallow( <Accordion title="Section" onToggle={ finishTest }>Content</Accordion> );

simulateTouchTap( wrapper );
simulateClick( wrapper );

function finishTest( isExpanded ) {
expect( isExpanded ).to.be.true;
Expand All @@ -80,11 +69,11 @@ describe( 'Accordion', function() {
} );

it( 'should always use the initialExpanded prop, if specified', function( done ) {
const wrapper = mount(
const wrapper = shallow(
<Accordion initialExpanded={ true } title="Section" onToggle={ finishTest }>Content</Accordion>
);

simulateTouchTap( wrapper );
simulateClick( wrapper );

function finishTest( isExpanded ) {
expect( isExpanded ).to.be.false;
Expand Down
2 changes: 1 addition & 1 deletion client/components/mobile-back-to-sidebar/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const MobileBackToSidebar = React.createClass( {

render: function() {
return (
<div className="mobile-back-to-sidebar" onTouchTap={ this.toggleSidebar }>
<div className="mobile-back-to-sidebar" onClick={ this.toggleSidebar }>
<svg className="gridicon gridicon-back-arrow mobile-back-to-sidebar__icon" height="24" width="24" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"><g><path d="M8.886 4L7 5.886 13.114 12 7 18.114 8.886 20l8-8"/></g></svg>
<span className="mobile-back-to-sidebar__content">
{ this.props.children }
Expand Down
4 changes: 2 additions & 2 deletions client/components/search/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ const Search = React.createClass( {
<div
className="search__icon-navigation"
ref="openIcon"
onTouchTap={ enableOpenIcon ? this.openSearch : this.focus }
onClick={ enableOpenIcon ? this.openSearch : this.focus }
tabIndex={ enableOpenIcon ? '0' : null }
onKeyDown={ enableOpenIcon
? this.openListener
Expand Down Expand Up @@ -385,7 +385,7 @@ const Search = React.createClass( {
return (
<div
className="search__icon-navigation"
onTouchTap={ this.closeSearch }
onClick={ this.closeSearch }
tabIndex="0"
onKeyDown={ this.closeListener }
aria-controls={ 'search-component-' + this.state.instanceId }
Expand Down
2 changes: 1 addition & 1 deletion client/components/section-nav/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ var SectionNav = React.createClass( {
<div className={ className }>
<div
className="section-nav__mobile-header"
onTouchTap={ this.toggleMobileOpenState }
onClick={ this.toggleMobileOpenState }
>
<span className="section-nav__mobile-header-text">
{ this.props.selectedText }
Expand Down
2 changes: 1 addition & 1 deletion client/components/section-nav/item.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var NavItem = React.createClass( {
href={ this.props.path }
target={ target }
className={ 'section-nav-' + itemClassPrefix + '__link' }
onTouchTap={ onClick }
onClick={ onClick }
tabIndex={ this.props.tabIndex || 0 }
aria-selected={ this.props.selected }
disabled={ this.props.disabled }
Expand Down
9 changes: 4 additions & 5 deletions client/components/section-nav/test/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ describe( 'section-nav', function() {
ReactDom = require( 'react-dom' );
React = require( 'react' );
TestUtils = require( 'react-addons-test-utils' );
require( 'react-tap-event-plugin' )();

const EMPTY_COMPONENT = require( 'test/helpers/react/empty-component' );

Expand Down Expand Up @@ -85,7 +84,7 @@ describe( 'section-nav', function() {
}, ( <p>placeholder</p> ) );
const tree = TestUtils.renderIntoDocument( elem );
assert( ! tree.state.mobileOpen );
TestUtils.Simulate.touchTap( ReactDom.findDOMNode(
TestUtils.Simulate.click( ReactDom.findDOMNode(
TestUtils.findRenderedDOMComponentWithClass( tree, 'section-nav__mobile-header' )
) );
assert( tree.state.mobileOpen );
Expand All @@ -100,15 +99,15 @@ describe( 'section-nav', function() {
const tree = TestUtils.renderIntoDocument( elem );

assert( ! tree.state.mobileOpen );
TestUtils.Simulate.touchTap( ReactDom.findDOMNode(
TestUtils.Simulate.click( ReactDom.findDOMNode(
TestUtils.findRenderedDOMComponentWithClass( tree, 'section-nav__mobile-header' )
) );
assert( tree.state.mobileOpen );
TestUtils.Simulate.touchTap( ReactDom.findDOMNode(
TestUtils.Simulate.click( ReactDom.findDOMNode(
TestUtils.findRenderedDOMComponentWithClass( tree, 'section-nav__mobile-header' )
) );
assert( ! tree.state.mobileOpen );
TestUtils.Simulate.touchTap( ReactDom.findDOMNode(
TestUtils.Simulate.click( ReactDom.findDOMNode(
TestUtils.findRenderedDOMComponentWithClass( tree, 'section-nav__mobile-header' )
) );
assert( tree.state.mobileOpen );
Expand Down
4 changes: 2 additions & 2 deletions client/components/segmented-control/item.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var SegmentedControlItem = React.createClass( {
} );

const linkClassName = classNames( 'segmented-control__link', {
[ `item-index-${this.props.index}` ]: this.props.index != null,
[ `item-index-${ this.props.index }` ]: this.props.index != null,
} );

return (
Expand All @@ -39,7 +39,7 @@ var SegmentedControlItem = React.createClass( {
href={ this.props.path }
className={ linkClassName }
ref="itemLink"
onTouchTap={ this.props.onClick }
onClick={ this.props.onClick }
title={ this.props.title }
role="radio"
tabIndex={ 0 }
Expand Down
2 changes: 1 addition & 1 deletion client/components/sidebar-navigation/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class SidebarNavigation extends React.Component {
render() {
return (
<header className="current-section">
<a onTouchTap={ this.toggleSidebar } className={ this.props.linkClassName }>
<a onClick={ this.toggleSidebar } className={ this.props.linkClassName }>
<Gridicon icon="chevron-left" />
{ this.props.children }
<div>
Expand Down
2 changes: 1 addition & 1 deletion client/my-sites/all-sites/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default React.createClass( {
href={ this.props.href }
onMouseEnter={ this.props.onMouseEnter }
onMouseLeave={ this.props.onMouseLeave }
onTouchTap={ this.onSelect }>
onClick={ this.onSelect }>
{ this.props.showCount && this.renderSiteCount() }
<div className="site__info">
<span className="site__title">{ title }</span>
Expand Down
1 change: 0 additions & 1 deletion client/my-sites/current-site/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ const CurrentSite = React.createClass( {
site={ selectedSite }
homeLink={ true }
externalLink={ true }
onClick={ this.previewSite }
onSelect={ this.previewSite }
tipTarget="site-card-preview" />
: <AllSites sites={ this.props.sites.get() } />
Expand Down
2 changes: 1 addition & 1 deletion client/my-sites/menus/menu-name.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ var MenuName = React.createClass( {
);
} else {
menuEditable = (
<span className={ this.props.className } onTouchTap={ this.toggleEdit }>
<span className={ this.props.className } onClick={ this.toggleEdit }>
<span>{ this.state.value }</span>
<a className="edit" />
</span>
Expand Down
2 changes: 1 addition & 1 deletion client/reader/list-gap/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var Gap = React.createClass( {
} );

return (
<div className={ classes } onTouchTap={ this.handleClick } >
<div className={ classes } onClick={ this.handleClick } >
<button type="button" className="button reader-list-gap__button">{ this.translate( 'Load More Posts' ) }</button>
</div>
);
Expand Down
7 changes: 1 addition & 6 deletions client/reader/share/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,6 @@ const ReaderShare = React.createClass( {
this._deferMenuChange( ! this.state.showingMenu );
},

killClick( event ) {
event.preventDefault();
},

closeMenu() {
// have to defer this to let the mouseup / click escape.
// If we don't defer and remove the DOM node on this turn of the event loop,
Expand Down Expand Up @@ -167,8 +163,7 @@ const ReaderShare = React.createClass( {

return React.createElement( this.props.tagName, {
className: 'reader-share',
onClick: this.killClick,
onTouchTap: this.toggle,
onClick: this.toggle,
onTouchStart: this.preloadEditor,
onMouseEnter: this.preloadEditor,
ref: 'shareButton' },
Expand Down
2 changes: 1 addition & 1 deletion client/reader/update-notice/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const UpdateNotice = React.createClass( {
} );

return (
<div className={ counterClasses } onTouchTap={ this.handleClick } >
<div className={ counterClasses } onClick={ this.handleClick } >
<DocumentHead unreadCount={ this.props.count } />
<Gridicon icon="arrow-up" size={ 18 } />
{ this.translate( '%s new post', '%s new posts', { args: [ this.props.cappedUnreadCount ], count: this.props.count } ) }
Expand Down
7 changes: 5 additions & 2 deletions docs/react-component-unit-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ When a user for example clicks an element does the component react like it shoul
Example test from `client/components/Accordion`

```javascript
var tree = TestUtils.renderIntoDocument( <Accordion title="Section" onToggle={ finishTest }>Content</Accordion> );
import { shallow } from 'enzyme';

TestUtils.Simulate.touchTap( ReactDom.findDOMNode( TestUtils.findRenderedDOMComponentWithClass( tree, 'accordion__toggle' ) ) );
const wrapper = shallow( <Accordion title="Section" onToggle={ finishTest }>Content</Accordion> );

wrapper.find( '.accordion__toggle' ).simulate( 'click' );
);

function finishTest( isExpanded ) {
expect( isExpanded ).to.be.ok;
Expand Down
Loading

0 comments on commit 1ca3a61

Please sign in to comment.