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.

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

Closed
wants to merge 10 commits into from
6 changes: 6 additions & 0 deletions docs/manifest.json
Expand Up @@ -695,6 +695,12 @@
"markdown_source": "../packages/components/src/date-time/README.md",
"parent": "components"
},
{
"title": "DetectFocusOutside",
Copy link
Contributor

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

Copy link
Member

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.

Copy link
Member Author

@aduth aduth Mar 16, 2020

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.

"slug": "detect-focus-outside",
"markdown_source": "../packages/components/src/detect-focus-outside/README.md",
"parent": "components"
},
{
"title": "DimensionControl",
"slug": "dimension-control",
Expand Down
36 changes: 36 additions & 0 deletions packages/components/src/detect-focus-outside/README.md
@@ -0,0 +1,36 @@
# DetectFocusOutside
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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. useResizeAware returns an element which is expected to be rendered as a child of a component. We need to capture events bubbling from within, so it would at least need to be a wrapping element, which would basically be the same as what this implementation is already.

I do think it would be ideal if we could at least flatten the DOM hierarchy, since currently this component must add a div wrapper. We don't strictly need the div, aside from being able to capture bubbling events. I recall having some hope that in the future, React might support event handlers to be assigned to Fragment, since their synthetic events don't use the DOM anyways and it can technically be possible.

In fact, they still note this in their documentation:

key is the only attribute that can be passed to Fragment. In the future, we may add support for additional attributes, such as event handlers.

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:

  • We could have a hook which returns an object of props for the event handlers. But how do we handle a case where both the hook and the original component each need to have an e.g. onFocus event handler?
  • We could have a hook which returns a ref, and then the hook attaches DOM event handlers to the ref instance. But should we have any concern that those are DOM events, not React events, which behave (and bubble) slightly differently? We've had issues with this in the past for things like Slot/Fill bubblesVirtually.

One possible advantage of using the DOM events is that we could use the actual blur event, where the distinction between blur and focusout is basically the point of what this component is trying to implement. As far as I can recall, React counter-intuitively uses the focusout event when assigning onBlur (see facebook/react#6410, specifically facebook/react#6410 (comment)).

Copy link
Contributor

Choose a reason for hiding this comment

The 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:

useSomething = ( ref ) => {
   useEffect(() => {
     // do something with ref.current.
     // useEffect ensures the ref is defined
  });
}

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 a component tree.

This is useful to detect whether focus has transitioned to another element outside a particular component tree, in which case DetectFocusOutside will invoke the onFocusOutside callback. This is unlike React's `onBlur`, which will be invoked even when focus transitions to another element in the same component tree.

## 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.
161 changes: 161 additions & 0 deletions packages/components/src/detect-focus-outside/index.js
@@ -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;
119 changes: 119 additions & 0 deletions packages/components/src/detect-focus-outside/test/index.js
@@ -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();
} );
} );
@@ -1,3 +1,5 @@
>⚠️ **Note:** While this higher-order component is not officially deprecated, you should consider using the [`DetectFocusOutside` component](../../detect-focus-outside) instead. This higher-order component _does not_ and will never support being used by function components.

# withFocusOutside

`withFocusOutside` is a React [higher-order component](https://facebook.github.io/react/docs/higher-order-components.html) to enable behavior to occur when focus leaves an element. Since a `blur` event will fire in React even when focus transitions to another element in the same context, this higher-order component encapsulates the logic necessary to determine if focus has truly left the element.
Expand Down