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

Editor: Fix tags bug by using correct container for blur event #2054

Merged
merged 3 commits into from
Jan 5, 2016
Merged

Editor: Fix tags bug by using correct container for blur event #2054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca9c572
Use correct container for blur event
nylen Jan 1, 2016
43c22bb
Fix more blur issues in Firefox and IE
nylen Jan 4, 2016
e4f233f
Fix failing tests
nylen Jan 4, 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
23 changes: 19 additions & 4 deletions client/components/token-field/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,18 @@ var TokenField = React.createClass( {
} );

return (
<div className={ classes } tabIndex="-1" onKeyDown={ this._onKeyDown } onBlur={ this._onBlur } onFocus={ this._onFocus }>
<div ref="tokensAndInput" className="token-field__input-container" onClick={ this._onClick } tabIndex="-1">
<div ref="main"
className={ classes }
tabIndex="-1"
onKeyDown={ this._onKeyDown }
onBlur={ this._onBlur }
onFocus={ this._onFocus }
>
<div ref="tokensAndInput"
className="token-field__input-container"
onClick={ this._onClick }
tabIndex="-1"
>
{ this._renderTokensAndInput() }
</div>
<SuggestionsList
Expand All @@ -81,7 +91,8 @@ var TokenField = React.createClass( {
scrollIntoView={ this.state.selectedSuggestionScroll }
isExpanded={ this.state.isActive }
onHover={ this._onSuggestionHovered }
onSelect={ this._onSuggestionSelected } />
onSelect={ this._onSuggestionSelected }
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I just had the linter warn me on a branch that the closing tag should be on the same line as the last prop. This was "new" to me, but figured I'd mention it here too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just had the linter warn me on a branch that the closing tag should be on the same line as the last prop.

I believe this only applies to self-closing tags, and was added in #1807.

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'm strongly in favor of putting the /> (and even the > for tags with children) on the following line, de-indented 1 level:

  • Like </div> ); } etc. that often appear immediately below, it denotes the end of a block
  • Easier to rearrange props, like trailing , in arrays

We have lots of code in both styles in Calypso. If we want to do something about this, let's do it in a separate PR and either remove that linter rule or update our code to this style.

</div>
);
},
Expand Down Expand Up @@ -127,7 +138,11 @@ var TokenField = React.createClass( {
},

_onBlur: function( event ) {
var stillActive = event.target.contains( event.relatedTarget );
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' );
Expand Down
1 change: 1 addition & 0 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 @@ -270,6 +270,7 @@ describe( 'TokenField', function() {
expect( wrapper.refs.tokenField.state.isActive ).to.equal( isActive );
}

document.activeElement = document.body;
TestUtils.Simulate.blur( textInputNode );
setTimeout( function() {
// After blur, need to wait for TokenField#_blurTimeoutID
Expand Down