-
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
Editor: PostTitle: Decode entities for display #20887
Changes from 9 commits
e14f981
bece56c
9fc08cb
bec7518
df174b9
9eadc8e
d80332d
5993c24
9da67b9
65c6232
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,36 @@ | ||
# DetectFocusOutside | ||
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. Lately, I have been wondering if it's better to avoid components for these "behavior-based" components and instead write hooks that accept "refs" or return elements (like useResizeAware). The advantage being to avoid the extra wrapper. 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. Hm, I'm not sure exactly how that would look in this case. I do think it would be ideal if we could at least flatten the DOM hierarchy, since currently this component must add a In fact, they still note this in their documentation:
https://reactjs.org/docs/fragments.html#keyed-fragments I think I saw later that they might be avoiding that option though (perhaps part of their new "Fire" effort). So maybe it's not worth placing our bets on this. The alternatives might have some concerning implication on the ergonomics of the component:
One possible advantage of using the DOM events is that we could use the actual 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. I'm not sure if a good pattern or not but on previous occasions I've wrote hooks like that:
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. I believe it would be good if we find a pattern here and follow it for future hooks relying on refs. 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. I think the harder question is if and when we "rely on refs", including whether it makes sense here. |
||
|
||
`DetectFocusOutside` is a component used to help detect when focus leaves an element. | ||
|
||
This can be useful to disambiguate focus transitions within the same element. A React `blur` event will fire even when focus transitions to another element in the same context. `DetectFocusOutside` will only invoke its callback prop when focus has truly left the element. | ||
aduth marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Usage | ||
|
||
Wrap your rendered children with `DetectFocusOutside`, defining an `onFocusOutside` callback prop. | ||
|
||
```jsx | ||
import { DetectFocusOutside, TextControl } from '@wordpress/components'; | ||
|
||
function MyComponent() { | ||
function onFocusOutside() { | ||
console.log( 'Focus has left the rendered element.' ); | ||
} | ||
|
||
return ( | ||
<DetectFocusOutside onFocusOutside={ onFocusOutside }> | ||
<TextControl /> | ||
<TextControl /> | ||
</DetectFocusOutside> | ||
); | ||
); | ||
``` | ||
|
||
In the above example, the `onFocusOutside` function is only called if focus leaves the element, and not if transitioning focus between the two inputs. | ||
|
||
## Props | ||
|
||
### `onFocusOutside` | ||
* **Type:** `Function` | ||
* **Required:** Yes | ||
|
||
A callback function to invoke when focus has left the rendered element. The callback will receive the original blur event. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useRef, useEffect } from '@wordpress/element'; | ||
|
||
/** @typedef {import('react').MouseEvent} ReactMouseEvent */ | ||
/** @typedef {import('react').TouchEvent} ReactTouchEvent */ | ||
/** @typedef {import('react').SyntheticEvent} ReactSyntheticEvent */ | ||
/** @typedef {import('react').ReactNode} ReactNode */ | ||
|
||
/** | ||
* @typedef {(event:ReactSyntheticEvent)=>void} OnFocusOutsideProp | ||
*/ | ||
|
||
/** | ||
* Input types which are classified as button types, for use in considering | ||
* whether element is a (focus-normalized) button. | ||
* | ||
* @type {Set<string>} | ||
*/ | ||
const INPUT_BUTTON_TYPES = new Set( [ 'button', 'submit' ] ); | ||
|
||
/** | ||
* Event types which are classified as ends of interaction considered for focus | ||
* event normalization. | ||
* | ||
* @type {Set<string>} | ||
*/ | ||
const INTERACTION_END_TYPES = new Set( [ 'mouseup', 'touchend' ] ); | ||
|
||
/** | ||
* Returns true if the given element is a button element subject to focus | ||
* normalization, or false otherwise. | ||
* | ||
* @see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus | ||
* | ||
* @param {Element} element Element to test. | ||
* | ||
* @return {boolean} Whether element is a button. | ||
*/ | ||
function isFocusNormalizedButton( element ) { | ||
switch ( element.nodeName ) { | ||
case 'A': | ||
case 'BUTTON': | ||
return true; | ||
|
||
case 'INPUT': | ||
const { type } = /** @type {HTMLInputElement} */ ( element ); | ||
return INPUT_BUTTON_TYPES.has( type ); | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* Component used to help detect when focus leaves an element. | ||
* | ||
* This can be useful to disambiguate focus transitions within the same element. | ||
* A React `blur` event will fire even when focus transitions to another element | ||
* in the same context. `DetectFocusOutside` will only invoke its callback prop | ||
* when focus has truly left the element | ||
* | ||
* @type {import('react').FC} | ||
* | ||
* @param {Object} props Component props. | ||
* @param {OnFocusOutsideProp} props.onFocusOutside Callback function to invoke | ||
* when focus has left the | ||
* rendered element. The | ||
* callback will receive the | ||
* original blur event. | ||
* @param {ReactNode} props.children Element children. | ||
*/ | ||
function DetectFocusOutside( { children, onFocusOutside } ) { | ||
const blurCheckTimeout = /** @type {import('react').MutableRefObject<number>} */ ( useRef() ); | ||
const preventBlurCheck = useRef( false ); | ||
useEffect( () => cancelBlurCheck ); | ||
|
||
/** | ||
* Cancels the scheduled blur check, if one is scheduled. | ||
*/ | ||
function cancelBlurCheck() { | ||
clearTimeout( blurCheckTimeout.current ); | ||
} | ||
|
||
/** | ||
* Schedules a blur check to occur after the current call stack ends, during | ||
* which time it may be expected that the check is nullified based on other | ||
* interactions. | ||
* | ||
* @see normalizeButtonFocus | ||
* | ||
* @param {ReactSyntheticEvent} event Focus event. | ||
*/ | ||
function queueBlurCheck( event ) { | ||
// Skip blur check if clicking button. See `normalizeButtonFocus`. | ||
if ( preventBlurCheck.current ) { | ||
return; | ||
} | ||
|
||
// React does not allow using an event reference asynchronously due to | ||
// recycling behavior, except when explicitly persisted. | ||
event.persist(); | ||
|
||
blurCheckTimeout.current = setTimeout( () => { | ||
// If document is not focused then focus should remain inside the | ||
// wrapped component and therefore we cancel this blur event thereby | ||
// leaving focus in place. | ||
// | ||
// See: https://developer.mozilla.org/en-US/docs/Web/API/Document/hasFocus | ||
if ( document.hasFocus() ) { | ||
onFocusOutside( event ); | ||
} | ||
}, 0 ); | ||
} | ||
|
||
/** | ||
* Handles a mousedown or mouseup event to respectively assign and unassign | ||
* a flag for preventing blur check on button elements. Some browsers, | ||
* namely Firefox and Safari, do not emit a focus event on button elements | ||
* when clicked, while others do. The logic here intends to normalize this | ||
* as treating click on buttons as focus. | ||
* | ||
* @see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus | ||
* | ||
* @param {ReactMouseEvent|ReactTouchEvent} event Event on which to base | ||
* normalize. | ||
*/ | ||
function normalizeButtonFocus( event ) { | ||
const { type, target } = event; | ||
if ( ! ( target instanceof window.Element ) ) { | ||
return; | ||
} | ||
|
||
const isInteractionEnd = INTERACTION_END_TYPES.has( type ); | ||
if ( isInteractionEnd ) { | ||
preventBlurCheck.current = false; | ||
} else if ( isFocusNormalizedButton( target ) ) { | ||
preventBlurCheck.current = true; | ||
} | ||
} | ||
|
||
// Disable reason: See `normalizeButtonFocus` for browser-specific focus | ||
// event normalization. | ||
|
||
/* eslint-disable jsx-a11y/no-static-element-interactions */ | ||
return ( | ||
<div | ||
onFocus={ cancelBlurCheck } | ||
onMouseDown={ normalizeButtonFocus } | ||
onMouseUp={ normalizeButtonFocus } | ||
onTouchStart={ normalizeButtonFocus } | ||
onTouchEnd={ normalizeButtonFocus } | ||
onBlur={ queueBlurCheck } | ||
> | ||
{ children } | ||
</div> | ||
); | ||
/* eslint-enable jsx-a11y/no-static-element-interactions */ | ||
} | ||
|
||
export default DetectFocusOutside; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import TestUtils from 'react-dom/test-utils'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { Component } from '@wordpress/element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import ReactDOM from 'react-dom'; | ||
import DetectFocusOutside from '../'; | ||
|
||
let wrapper, onFocusOutside; | ||
|
||
describe( 'DetectFocusOutside', () => { | ||
let origHasFocus; | ||
|
||
class TestComponent extends Component { | ||
render() { | ||
return ( | ||
<DetectFocusOutside | ||
onFocusOutside={ this.props.onFocusOutside } | ||
> | ||
<input /> | ||
<input type="button" /> | ||
</DetectFocusOutside> | ||
); | ||
} | ||
} | ||
|
||
const simulateEvent = ( event, index = 0 ) => { | ||
const element = TestUtils.scryRenderedDOMComponentsWithTag( | ||
wrapper, | ||
'input' | ||
); | ||
TestUtils.Simulate[ event ]( element[ index ] ); | ||
}; | ||
|
||
beforeEach( () => { | ||
// Mock document.hasFocus() to always be true for testing | ||
// note: we overide this for some tests. | ||
origHasFocus = document.hasFocus; | ||
document.hasFocus = () => true; | ||
|
||
onFocusOutside = jest.fn(); | ||
wrapper = TestUtils.renderIntoDocument( | ||
<TestComponent onFocusOutside={ onFocusOutside } /> | ||
); | ||
} ); | ||
|
||
afterEach( () => { | ||
document.hasFocus = origHasFocus; | ||
} ); | ||
|
||
it( 'should not call handler if focus shifts to element within component', () => { | ||
simulateEvent( 'focus' ); | ||
simulateEvent( 'blur' ); | ||
simulateEvent( 'focus', 1 ); | ||
|
||
jest.runAllTimers(); | ||
|
||
expect( onFocusOutside ).not.toHaveBeenCalled(); | ||
} ); | ||
|
||
it( 'should not call handler if focus transitions via click to button', () => { | ||
simulateEvent( 'focus' ); | ||
simulateEvent( 'mouseDown', 1 ); | ||
simulateEvent( 'blur' ); | ||
|
||
// In most browsers, the input at index 1 would receive a focus event | ||
// at this point, but this is not guaranteed, which is the intention of | ||
// the normalization behavior tested here. | ||
simulateEvent( 'mouseUp', 1 ); | ||
|
||
jest.runAllTimers(); | ||
|
||
expect( onFocusOutside ).not.toHaveBeenCalled(); | ||
} ); | ||
|
||
it( 'should call handler if focus doesn’t shift to element within component', () => { | ||
simulateEvent( 'focus' ); | ||
simulateEvent( 'blur' ); | ||
|
||
jest.runAllTimers(); | ||
|
||
expect( onFocusOutside ).toHaveBeenCalled(); | ||
} ); | ||
|
||
it( 'should not call handler if focus shifts outside the component when the document does not have focus', () => { | ||
// Force document.hasFocus() to return false to simulate the window/document losing focus | ||
// See https://developer.mozilla.org/en-US/docs/Web/API/Document/hasFocus. | ||
document.hasFocus = () => false; | ||
|
||
simulateEvent( 'focus' ); | ||
simulateEvent( 'blur' ); | ||
|
||
jest.runAllTimers(); | ||
|
||
expect( onFocusOutside ).not.toHaveBeenCalled(); | ||
} ); | ||
|
||
it( 'should cancel check when unmounting while queued', () => { | ||
simulateEvent( 'focus' ); | ||
simulateEvent( 'input' ); | ||
|
||
ReactDOM.unmountComponentAtNode( | ||
// eslint-disable-next-line react/no-find-dom-node | ||
ReactDOM.findDOMNode( wrapper ).parentNode | ||
); | ||
|
||
jest.runAllTimers(); | ||
|
||
expect( onFocusOutside ).not.toHaveBeenCalled(); | ||
} ); | ||
} ); |
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.
This seems useful for the Gallery block. I know we merged a similar thing to unselect images lately cc @nosolosw
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.
Oh, nice. Yeah, it looks like the same use case (link for reference). Note that I didn't extract this logic to a reusable component there, so we can refactor gallery to use this when the PR lands.
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.
@nosolosw FWIW, there is the
withFocusOutside
higher-order component already available today:https://github.com/WordPress/gutenberg/tree/master/packages/components/src/higher-order/with-focus-outside
This pull request "discourages" it, but it will expect to continue to exist I think. Maybe still worth waiting, in case we want to consolidate to
DetectFocusOutside
as the encouraged implementation.