-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
Title: Unselect title by blur event (B) #2974
Changes from all commits
ca44284
2974da7
ea8f8a1
8f2d430
e2150d4
d5f69c2
7f8c125
e30a814
5747cdc
f5dd96b
e3179f5
61b7608
9390fcc
6103641
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import hoistNonReactStatic from 'hoist-non-react-statics'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { Component, findDOMNode } from '@wordpress/element'; | ||
|
||
/* Heavily based on react-click-outside (https://github.com/kentor/react-click-outside/blob/master/index.js), | ||
* this Higher Order Component wraps a component and fires any handleFocusOutside listeners it might have | ||
* if a focus is detected ouside that component | ||
* | ||
* @param {WPElement} OriginalComponent the original component | ||
* | ||
* @return {Component} Component with focus outside detection | ||
*/ | ||
|
||
function withFocusOutside( OriginalComponent ) { | ||
const componentName = OriginalComponent.displayName || OriginalComponent.name; | ||
|
||
class EnhancedComponent extends Component { | ||
constructor() { | ||
super( ...arguments ); | ||
this.onFocusOutside = this.onFocusOutside.bind( this ); | ||
this.bindRef = this.bindRef.bind( this ); | ||
} | ||
|
||
componentDidMount() { | ||
document.addEventListener( 'focusin', this.onFocusOutside, true ); | ||
} | ||
|
||
componentWillUnmount() { | ||
document.removeEventListener( 'focusin', this.onFocusOutside, true ); | ||
} | ||
|
||
onFocusOutside( e ) { | ||
const domNode = this.__domNode; | ||
if ( | ||
( ! domNode || ! domNode.contains( e.target ) ) && | ||
typeof this.__wrappedInstance.handleFocusOutside === 'function' | ||
) { | ||
this.__wrappedInstance.handleFocusOutside( e ); | ||
} | ||
} | ||
|
||
bindRef( ref ) { | ||
this.__wrappedInstance = ref; | ||
// eslint-disable-next-line react/no-find-dom-node | ||
this.__domNode = findDOMNode( ref ); | ||
} | ||
|
||
render() { | ||
return ( | ||
<OriginalComponent | ||
{ ...this.props } | ||
ref={ this.bindRef } | ||
/> | ||
); | ||
} | ||
} | ||
|
||
EnhancedComponent.displayName = `FocusOutside(${ componentName })`; | ||
|
||
return hoistNonReactStatic( EnhancedComponent, OriginalComponent ); | ||
} | ||
|
||
export default withFocusOutside; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { shallow } from 'enzyme'; | ||
import { Component } from '../../../../element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import withFocusOutside from '../'; | ||
|
||
class Test extends Component { | ||
render() { | ||
return ( | ||
<div className="test">Testing</div> | ||
); | ||
} | ||
} | ||
|
||
describe( 'withFocusOutside()', () => { | ||
const Composite = withFocusOutside( Test ); | ||
|
||
it( 'should render a basic Test component inside the HOC', () => { | ||
const renderedComposite = shallow( <Composite /> ); | ||
const wrappedElement = renderedComposite.find( Test ); | ||
const wrappedElementShallow = wrappedElement.shallow(); | ||
expect( wrappedElementShallow.hasClass( 'test' ) ).toBe( true ); | ||
expect( wrappedElementShallow.type() ).toBe( 'div' ); | ||
expect( wrappedElementShallow.text() ).toBe( 'Testing' ); | ||
} ); | ||
|
||
it( 'should pass additional props through to the wrapped element', () => { | ||
const renderedComposite = shallow( <Composite test="test" /> ); | ||
const wrappedElement = renderedComposite.find( Test ); | ||
// Ensure that the wrapped Test element has the appropriate props. | ||
expect( wrappedElement.props().test ).toBe( 'test' ); | ||
} ); | ||
} ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ | |
*/ | ||
import { connect } from 'react-redux'; | ||
import Textarea from 'react-autosize-textarea'; | ||
import clickOutside from 'react-click-outside'; | ||
import classnames from 'classnames'; | ||
|
||
/** | ||
|
@@ -12,6 +11,7 @@ import classnames from 'classnames'; | |
import { __ } from '@wordpress/i18n'; | ||
import { Component } from '@wordpress/element'; | ||
import { keycodes } from '@wordpress/utils'; | ||
import { withFocusOutside } from '@wordpress/components'; | ||
|
||
/** | ||
* Internal dependencies | ||
|
@@ -35,8 +35,13 @@ class PostTitle extends Component { | |
this.onSelect = this.onSelect.bind( this ); | ||
this.onUnselect = this.onUnselect.bind( this ); | ||
this.onSelectionChange = this.onSelectionChange.bind( this ); | ||
this.onContainerFocus = this.onContainerFocus.bind( this ); | ||
this.setFocused = this.setFocused.bind( this ); | ||
this.focusText = this.focusText.bind( this ); | ||
this.handleFocusOutside = this.handleFocusOutside.bind( this ); | ||
this.state = { | ||
isSelected: false, | ||
hasFocusWithin: false, | ||
}; | ||
} | ||
|
||
|
@@ -62,6 +67,10 @@ class PostTitle extends Component { | |
} | ||
} | ||
|
||
focusText() { | ||
this.textareaContainer.textarea.focus(); | ||
} | ||
|
||
onChange( event ) { | ||
const newTitle = event.target.value.replace( REGEXP_NEWLINES, ' ' ); | ||
this.props.onUpdate( newTitle ); | ||
|
@@ -76,8 +85,16 @@ class PostTitle extends Component { | |
this.setState( { isSelected: false } ); | ||
} | ||
|
||
handleClickOutside() { | ||
this.setState( { isSelected: false } ); | ||
setFocused( focused ) { | ||
this.setState( { hasFocusWithin: focused } ); | ||
} | ||
|
||
handleFocusOutside() { | ||
this.setFocused( false ); | ||
} | ||
|
||
onContainerFocus() { | ||
this.setFocused( true ); | ||
} | ||
|
||
onKeyDown( event ) { | ||
|
@@ -88,12 +105,14 @@ class PostTitle extends Component { | |
|
||
render() { | ||
const { title } = this.props; | ||
const { isSelected } = this.state; | ||
const className = classnames( 'editor-post-title', { 'is-selected': isSelected } ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe isSelected should be something like isTyping? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The naming had confused me as well when I first encountered it. I think |
||
const { isSelected, hasFocusWithin } = this.state; | ||
const className = classnames( 'editor-post-title', { 'is-selected': isSelected && hasFocusWithin } ); | ||
|
||
return ( | ||
<div className={ className }> | ||
{ isSelected && <PostPermalink /> } | ||
<div | ||
className={ className } | ||
onFocus={ this.onContainerFocus }> | ||
{ isSelected && hasFocusWithin && <PostPermalink onLinkCopied={ this.focusText } /> } | ||
<h1> | ||
<Textarea | ||
ref={ this.bindTextarea } | ||
|
@@ -126,4 +145,4 @@ export default connect( | |
}, | ||
}; | ||
} | ||
)( clickOutside( PostTitle ) ); | ||
)( withFocusOutside( PostTitle ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be a separate component, or can we assign the button of
ClipboardButton
as thecontainer
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to have shouldComponentUpdate return false so that the container with the clipboard textarea isn't removed from the DOM. The problem is if it gets redrawn, then the textarea that is inside gets removed, and the clipboard library throws an error when trying to remove it. We just need a component that can get things added to it, and isn't going to get recreated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had it as the button originally, and kept getting an error in the clipboard library when it tried to remove the textarea, because react had already removed it because the button had been redrawn. You would get this error if you tried clicking on the button after you had already clicked on it. Therefore, the idea was to make a component that could have anything inside it, and react wouldn't keep trying to remove it. It's a limitation of using a third party library which appends components into the DOM. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't supply container, then it defaults to body, which is fine normally, but it will fire a blur which will trigger focus outside. That's why I'm supplying container ... to keep the focus inside the post title block.