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

[react-interactions] Refine custom active element blur logic #17354

Merged
merged 2 commits into from
Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,3 +465,7 @@ export function unmountFundamentalComponent(fundamentalInstance) {
export function getInstanceFromNode(node) {
throw new Error('Not yet implemented.');
}

export function beforeRemoveInstance(instance) {
// noop
}
67 changes: 49 additions & 18 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ export type ChildSet = void; // Unused
export type TimeoutHandle = TimeoutID;
export type NoTimeout = -1;

type SelectionInformation = {|
blurredActiveElement: null | HTMLElement,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
blurredActiveElement: null | HTMLElement,
detachedActiveElement: null | HTMLElement,

focusedElem: null | HTMLElement,
selectionRange: mixed,
|};

import {
enableSuspenseServerRenderer,
enableFlareAPI,
Expand All @@ -127,7 +133,7 @@ const SUSPENSE_FALLBACK_START_DATA = '$!';
const STYLE = 'style';

let eventsEnabled: ?boolean = null;
let selectionInformation: ?mixed = null;
let selectionInformation: null | SelectionInformation = null;

function shouldAutoFocusHostComponent(type: string, props: Props): boolean {
switch (type) {
Expand Down Expand Up @@ -205,6 +211,13 @@ export function prepareForCommit(containerInfo: Container): void {

export function resetAfterCommit(containerInfo: Container): void {
restoreSelection(selectionInformation);
if (enableFlareAPI) {
const blurredActiveElement = (selectionInformation: any)
.blurredActiveElement;
if (blurredActiveElement !== null) {
dispatchActiveElementBlur(blurredActiveElement);
}
Comment on lines +215 to +219
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const blurredActiveElement = (selectionInformation: any)
.blurredActiveElement;
if (blurredActiveElement !== null) {
dispatchActiveElementBlur(blurredActiveElement);
}
const activeElementDetached = (selectionInformation: any).activeElementDetached;
if (activeElementDetached !== null) {
dispatchDetachedBlur(activeElementDetached);
}

}
selectionInformation = null;
ReactBrowserEventEmitterSetEnabled(eventsEnabled);
eventsEnabled = null;
Expand Down Expand Up @@ -452,38 +465,57 @@ export function insertInContainerBefore(
}
}

function dispatchFlareDetachedBlurEvent(
elementDetached: boolean,
targetInstance: null | Object,
target: Element | Document,
): void {
// Simlulate the custom event to the React Flare responder system.
dispatchEventForResponderEventSystem(
'blur',
targetInstance,
({
elementDetached,
target,
timeStamp: Date.now(),
}: any),
target,
RESPONDER_EVENT_SYSTEM | IS_PASSIVE,
);
}

function dispatchBeforeActiveElementBlur(element: HTMLElement): void {
const targtInstance = getClosestInstanceFromNode(element);
((selectionInformation: any): SelectionInformation).blurredActiveElement = element;
dispatchFlareDetachedBlurEvent(false, targtInstance, element);
}
Comment on lines +487 to +491
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function dispatchBeforeActiveElementBlur(element: HTMLElement): void {
const targtInstance = getClosestInstanceFromNode(element);
((selectionInformation: any): SelectionInformation).blurredActiveElement = element;
dispatchFlareDetachedBlurEvent(false, targtInstance, element);
}
function dispatchBeforeDetachedBlur(target: HTMLElement): void {
const targtInstance = getClosestInstanceFromNode(element);
selectionInformation.detachedActiveElement = target;
dispatchEventForResponderEventSystem(
'beforeblur',
targetInstance,
({
target,
timeStamp: Date.now(),
}: any),
target,
RESPONDER_EVENT_SYSTEM | IS_PASSIVE,
);


function dispatchActiveElementBlur(
node: Instance | TextInstance | SuspenseInstance,
): void {
dispatchFlareDetachedBlurEvent(true, null, ((node: any): HTMLElement));
}
Comment on lines +493 to +497
Copy link
Contributor

@necolas necolas Nov 13, 2019

Choose a reason for hiding this comment

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

Suggested change
function dispatchActiveElementBlur(
node: Instance | TextInstance | SuspenseInstance,
): void {
dispatchFlareDetachedBlurEvent(true, null, ((node: any): HTMLElement));
}
function dispatchDetachedBlur(target) {
const targtInstance = getClosestInstanceFromNode(target);
dispatchEventForResponderEventSystem(
'blur',
targetInstance,
({
isTargetAttached: false,
target,
timeStamp: Date.now(),
}: any),
target,
RESPONDER_EVENT_SYSTEM | IS_PASSIVE,
);


// This is a specific event for the React Flare
// event system, so event responders can act
// accordingly to a DOM node being unmounted that
// previously had active document focus.
function dispatchDetachedVisibleNodeEvent(
child: Instance | TextInstance | SuspenseInstance,
export function beforeRemoveInstance(
instance: Instance | TextInstance | SuspenseInstance,
): void {
if (
enableFlareAPI &&
selectionInformation &&
child === selectionInformation.focusedElem
instance === selectionInformation.focusedElem
) {
const targetFiber = getClosestInstanceFromNode(child);
// Simlulate a blur event to the React Flare responder system.
dispatchEventForResponderEventSystem(
'detachedvisiblenode',
targetFiber,
({
target: child,
timeStamp: Date.now(),
}: any),
((child: any): Document | Element),
RESPONDER_EVENT_SYSTEM | IS_PASSIVE,
);
dispatchBeforeActiveElementBlur(((instance: any): HTMLElement));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dispatchBeforeActiveElementBlur(((instance: any): HTMLElement));
dispatchBeforeDetachedBlur(((instance: any): HTMLElement));

}
}

export function removeChild(
parentInstance: Instance,
child: Instance | TextInstance | SuspenseInstance,
): void {
dispatchDetachedVisibleNodeEvent(child);
parentInstance.removeChild(child);
}

Expand All @@ -494,7 +526,6 @@ export function removeChildFromContainer(
if (container.nodeType === COMMENT_NODE) {
(container.parentNode: any).removeChild(child);
} else {
dispatchDetachedVisibleNodeEvent(child);
container.removeChild(child);
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/react-dom/src/client/ReactInputSelection.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ export function hasSelectionCapabilities(elem) {
export function getSelectionInformation() {
const focusedElem = getActiveElementDeep();
return {
// Used by Flare
blurredActiveElement: null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
blurredActiveElement: null,
activeElementDetached: null,

focusedElem: focusedElem,
selectionRange: hasSelectionCapabilities(focusedElem)
? getSelection(focusedElem)
Expand Down
77 changes: 50 additions & 27 deletions packages/react-interactions/events/src/dom/Focus.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,28 +45,25 @@ type FocusProps = {
onFocusVisibleChange: boolean => void,
};

type FocusEventType =
| 'focus'
| 'blur'
| 'focuschange'
| 'focusvisiblechange'
| 'detachedvisiblenode';
type FocusEventType = 'focus' | 'blur' | 'focuschange' | 'focusvisiblechange';

type FocusWithinProps = {
disabled?: boolean,
onFocusWithin?: (e: FocusEvent) => void,
onBlurWithin?: (e: FocusEvent) => void,
onFocusWithinChange?: boolean => void,
onFocusWithinVisibleChange?: boolean => void,
onDetachedVisibleNode?: (e: FocusEvent) => void,
onBeforeFocusedElementDetached?: (e: FocusEvent) => void,
Copy link
Contributor

Choose a reason for hiding this comment

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

onBeforeBlurWithin, only called when the element is going to be removed from the document

onFocusedElementDetached?: (e: FocusEvent) => void,
Copy link
Contributor

Choose a reason for hiding this comment

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

This wouldn't be needed, onBlurWithin payload can include the elementAttached data

};

type FocusWithinEventType =
| 'focuswithinvisiblechange'
| 'focuswithinchange'
| 'blurwithin'
| 'focuswithin'
| 'detachedvisiblenode';
| 'focusedelementdetached'
| 'beforefocusedelementdetached';
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

Replaced with beforeblurwithin


/**
* Shared between Focus and FocusWithin
Expand All @@ -79,14 +76,22 @@ const isMac =
? /^Mac/.test(window.navigator.platform)
: false;

const targetEventTypes = ['focus', 'blur', 'detachedvisiblenode'];
const targetEventTypes = ['focus', 'blur'];

const hasPointerEvents =
typeof window !== 'undefined' && window.PointerEvent != null;

const rootEventTypes = hasPointerEvents
? ['keydown', 'keyup', 'pointermove', 'pointerdown', 'pointerup']
: ['keydown', 'keyup', 'mousedown', 'touchmove', 'touchstart', 'touchend'];
? ['keydown', 'keyup', 'pointermove', 'pointerdown', 'pointerup', 'blur']
: [
'keydown',
'keyup',
'mousedown',
'touchmove',
'touchstart',
'touchend',
'blur',
];

function isFunction(obj): boolean {
return typeof obj === 'function';
Expand Down Expand Up @@ -504,6 +509,23 @@ const focusWithinResponderImpl = {
break;
}
case 'blur': {
if ((nativeEvent: any).elementDetached === false) {
const onBeforeFocusedElementDetached = (props.onBeforeFocusedElementDetached: any);
if (isFunction(onBeforeFocusedElementDetached)) {
const syntheticEvent = createFocusEvent(
context,
'beforefocusedelementdetached',
event.target,
state.pointerType,
);
context.dispatchEvent(
syntheticEvent,
onBeforeFocusedElementDetached,
DiscreteEvent,
);
}
return;
}
if (
state.isFocused &&
!context.isTargetWithinResponder(relatedTarget)
Expand All @@ -514,22 +536,6 @@ const focusWithinResponderImpl = {
}
break;
}
case 'detachedvisiblenode': {
const onDetachedVisibleNode = (props.onDetachedVisibleNode: any);
if (isFunction(onDetachedVisibleNode)) {
const syntheticEvent = createFocusEvent(
context,
'detachedvisiblenode',
event.target,
state.pointerType,
);
context.dispatchEvent(
syntheticEvent,
onDetachedVisibleNode,
DiscreteEvent,
);
}
}
}
},
onRootEvent(
Expand All @@ -538,6 +544,23 @@ const focusWithinResponderImpl = {
props: FocusWithinProps,
state: FocusState,
): void {
if ((event.nativeEvent: any).elementDetached === true) {
const onFocusedElementDetached = (props.onFocusedElementDetached: any);
if (isFunction(onFocusedElementDetached)) {
const syntheticEvent = createFocusEvent(
context,
'focusedelementdetached',
event.target,
state.pointerType,
);
context.dispatchEvent(
syntheticEvent,
onFocusedElementDetached,
DiscreteEvent,
);
}
return;
}
handleRootEvent(event, context, state, isFocusVisible => {
if (state.isFocused && state.isFocusVisible !== isFocusVisible) {
state.isFocusVisible = isFocusVisible;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,37 +262,77 @@ describe.each(table)('FocusWithin responder', hasPointerEvents => {
});
});

describe('onDetachedVisibleNode', () => {
let onDetachedVisibleNode, ref, innerRef, innerRef2;

const Component = ({show}) => {
const listener = useFocusWithin({
onDetachedVisibleNode,
});
return (
<div ref={ref} listeners={listener}>
{show && <input ref={innerRef} />}
<div ref={innerRef2} />
</div>
);
};
describe('onBeforeFocusedElementDetached/onFocusedElementDetached', () => {
let onBeforeFocusedElementDetached,
onFocusedElementDetached,
ref,
innerRef,
innerRef2;

beforeEach(() => {
onDetachedVisibleNode = jest.fn();
onBeforeFocusedElementDetached = jest.fn();
onFocusedElementDetached = jest.fn();
ref = React.createRef();
innerRef = React.createRef();
innerRef2 = React.createRef();
ReactDOM.render(<Component show={true} />, container);
});

it('is called after a focused element is unmounted', () => {
const Component = ({show}) => {
const listener = useFocusWithin({
onBeforeFocusedElementDetached,
onFocusedElementDetached,
});
return (
<div ref={ref} listeners={listener}>
{show && <input ref={innerRef} />}
<div ref={innerRef2} />
</div>
);
};

ReactDOM.render(<Component show={true} />, container);

const inner = innerRef.current;
const target = createEventTarget(inner);
target.keydown({key: 'Tab'});
target.focus();
expect(onBeforeFocusedElementDetached).toHaveBeenCalledTimes(0);
expect(onFocusedElementDetached).toHaveBeenCalledTimes(0);
ReactDOM.render(<Component show={false} />, container);
expect(onBeforeFocusedElementDetached).toHaveBeenCalledTimes(1);
expect(onFocusedElementDetached).toHaveBeenCalledTimes(1);
});

it('is called after a nested focused element is unmounted', () => {
const Component = ({show}) => {
const listener = useFocusWithin({
onBeforeFocusedElementDetached,
onFocusedElementDetached,
});
return (
<div ref={ref} listeners={listener}>
{show && (
<div>
<input ref={innerRef} />
</div>
)}
<div ref={innerRef2} />
</div>
);
};

ReactDOM.render(<Component show={true} />, container);

const inner = innerRef.current;
const target = createEventTarget(inner);
target.keydown({key: 'Tab'});
target.focus();
expect(onDetachedVisibleNode).toHaveBeenCalledTimes(0);
expect(onBeforeFocusedElementDetached).toHaveBeenCalledTimes(0);
expect(onFocusedElementDetached).toHaveBeenCalledTimes(0);
ReactDOM.render(<Component show={false} />, container);
expect(onDetachedVisibleNode).toHaveBeenCalledTimes(1);
expect(onBeforeFocusedElementDetached).toHaveBeenCalledTimes(1);
expect(onFocusedElementDetached).toHaveBeenCalledTimes(1);
});
});

Expand Down
4 changes: 4 additions & 0 deletions packages/react-native-renderer/src/ReactFabricHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -495,3 +495,7 @@ export function cloneFundamentalInstance(fundamentalInstance) {
export function getInstanceFromNode(node) {
throw new Error('Not yet implemented.');
}

export function beforeRemoveInstance(instance) {
// noop
}
4 changes: 4 additions & 0 deletions packages/react-native-renderer/src/ReactNativeHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,3 +534,7 @@ export function unmountFundamentalComponent(fundamentalInstance) {
export function getInstanceFromNode(node) {
throw new Error('Not yet implemented.');
}

export function beforeRemoveInstance(instance) {
// noop
}
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ import {
updateFundamentalComponent,
commitHydratedContainer,
commitHydratedSuspenseInstance,
beforeRemoveInstance,
} from './ReactFiberHostConfig';
import {
captureCommitPhaseError,
Expand Down Expand Up @@ -808,6 +809,7 @@ function commitUnmount(
dependencies.responders = null;
}
}
beforeRemoveInstance(current.stateNode);
}
safelyDetachRef(current);
return;
Expand Down
Loading