Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Token Input: Fix Firefox blur problems #6162

Merged
merged 7 commits into from
Jun 30, 2016
Merged

Token Input: Fix Firefox blur problems #6162

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b1da1b5
Fix Firefox blur problems
marekhrabe Jun 20, 2016
cc524a7
Edit test to actually test what it describes
marekhrabe Jun 20, 2016
d3a03a7
Remove legacy timeout managment
marekhrabe Jun 20, 2016
03496d8
Remove unnecessary debug statements
marekhrabe Jun 22, 2016
59db858
Do not steal focus when accepting user input after blur
marekhrabe Jun 22, 2016
3b94338
Prevent clicking on the tokensAndInput container from adding a token
nylen Jun 27, 2016
0b1f0ef
Clean up tests
nylen Jun 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 10 additions & 72 deletions client/components/token-field/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,6 @@ var TokenField = React.createClass( {
}
},

componentWillUnmount: function() {
debug( 'componentWillUnmount' );
this._clearBlurTimeout();
},

render: function() {
var classes = classNames( 'token-field', {
'is-active': this.state.isActive,
Expand All @@ -116,7 +111,6 @@ var TokenField = React.createClass( {
tokenFieldProps = Object.assign( {}, tokenFieldProps, {
onKeyDown: this._onKeyDown,
onKeyPress: this._onKeyPress,
onBlur: this._onBlur,
onFocus: this._onFocus
} );
}
Expand All @@ -125,7 +119,6 @@ var TokenField = React.createClass( {
<div { ...tokenFieldProps } >
<div ref="tokensAndInput"
className="token-field__input-container"
onClick={ ! this.props.disabled && this._onClick }
tabIndex="-1"
>
{ this._renderTokensAndInput() }
Expand Down Expand Up @@ -178,7 +171,8 @@ var TokenField = React.createClass( {
ref: 'input',
key: 'input',
disabled: this.props.disabled,
value: this.state.incompleteTokenValue
value: this.state.incompleteTokenValue,
onBlur: this._onBlur,
};

if ( ! ( maxLength && value.length >= maxLength ) ) {
Expand All @@ -193,78 +187,22 @@ var TokenField = React.createClass( {
},

_onFocus: function( event ) {
if ( this._blurTimeoutID ) {
this._clearBlurTimeout();
return;
} else if ( this.state.isActive ) {
return; // already active
}

this.setState( { isActive: true } );
if ( 'function' === typeof this.props.onFocus ) {
this.props.onFocus( event );
}
},

_onBlur: function( event ) {
var blurSource = event.relatedTarget ||
event.nativeEvent.explicitOriginalTarget || // FF
document.activeElement; // IE11

var stillActive = this.refs.main.contains( blurSource );

if ( stillActive ) {
debug( '_onBlur but component still active; not doing anything' );
return; // we didn't leave the component, so don't do anything
}

debug( '_onBlur before timeout setting component inactive' );
this.setState( {
isActive: false
} );
/* When the component blurs, we need to add the current text, or
* the selected suggestion (if any).
*
* Two reasons to set a timeout rather than do this immediately:
* - Some other user action (like tapping on a suggestion) may
* have caused this blur. If there is another user-triggered
* event, we need to give it a chance to complete first.
* - At one point, using the right arrow key to move the text
* input was causing a blur to outside the component?! (left
* arrow key does not do this). So, we delay the resetting of
* the state and cancel it if we get focus back quick enough.
*/
debug( '_onBlur waiting to add current token' );
this._blurTimeoutID = window.setTimeout( function() {
// Add the current token, UNLESS the text input is empty and
// there is a suggested token selected. In that case, we don't
// want to add it, because it's easy to inadvertently hover
// over a suggestion.
if ( this._inputHasValidValue() ) {
debug( '_onBlur after timeout adding current token' );
this._addCurrentToken();
} else {
debug( '_onBlur after timeout not adding current token' );
}
debug( '_onBlur resetting component state' );
this.setState( this.getInitialState() );
this._clearBlurTimeout();
}.bind( this ), 0 );
},

_clearBlurTimeout: function() {
if ( this._blurTimeoutID ) {
debug( '_blurTimeoutID cleared' );
window.clearTimeout( this._blurTimeoutID );
this._blurTimeoutID = null;
}
},

_onClick: function() {
if ( this.refs.input && ! this.refs.input.hasFocus() ) {
debug( '_onClick focusing input' );
this.refs.input.focus();
debug( '_onBlur setting component inactive' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the new _onBlur code is very simple we can remove this debug statement...

if ( this._inputHasValidValue() ) {
debug( '_onBlur adding current token' );
this._addCurrentToken();
} else {
debug( '_onBlur not adding current token' );
}
debug( '_onBlur resetting component state' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and we can remove this one as well.

this.setState( this.getInitialState() );
},

_onTokenClickRemove: function( event ) {
Expand Down
6 changes: 6 additions & 0 deletions client/components/token-field/suggestions-list.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ var SuggestionsList = React.createClass( {
<li
className={ classes }
key={ suggestion }
onMouseDown={ this._handleMouseDown }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need onTouchStart here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this on the iPhone and mouseDown seems to do the job.

onClick={ this._handleClick( suggestion ) }
onMouseEnter={ this._handleHover( suggestion ) }>
{ match ?
Expand Down Expand Up @@ -124,6 +125,11 @@ var SuggestionsList = React.createClass( {
return function() {
this.props.onSelect( suggestion );
}.bind( this );
},

_handleMouseDown: function( e ) {
// By preventing default here, we will not lose focus of <input> when clicking a suggestion
e.preventDefault();
}
} );

Expand Down
5 changes: 2 additions & 3 deletions client/components/token-field/test/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,8 @@ describe( 'TokenField', function() {
it( 'should not lose focus when a suggestion is clicked', test( function() {
// prevents regression of https://github.com/Automattic/wp-calypso/issues/1884

textInputNode.simulate( 'blur', {
relatedTarget: document.querySelector( '.token-field__suggestion' )
} );
const firstSuggestion = tokenFieldNode.find( '.token-field__suggestion' ).at( 0 );
firstSuggestion.simulate( 'click' );

// wait for setState call
this.clock.tick( 10 );
Expand Down