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 1 commit
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
}
70 changes: 52 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,60 @@ export function insertInContainerBefore(
}
}

function dispatchCustomFlareEvent(
type: string,
targetInstance: null | Object,
target: Element | Document,
): void {
// Simlulate the custom event to the React Flare responder system.
dispatchEventForResponderEventSystem(
type,
targetInstance,
({
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;
dispatchCustomFlareEvent('beforeactiveelementblur', targtInstance, element);
}

function dispatchActiveElementBlur(
node: Instance | TextInstance | SuspenseInstance,
): void {
dispatchCustomFlareEvent(
'activeelementblur',
null,
((node: any): HTMLElement),
);
}

// 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 +529,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
57 changes: 46 additions & 11 deletions packages/react-interactions/events/src/dom/Focus.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,26 @@ type FocusEventType =
| 'blur'
| 'focuschange'
| 'focusvisiblechange'
| 'detachedvisiblenode';
| 'beforeactiveelementblur'
trueadm marked this conversation as resolved.
Show resolved Hide resolved
| 'activeelementblur';

type FocusWithinProps = {
disabled?: boolean,
onFocusWithin?: (e: FocusEvent) => void,
onBlurWithin?: (e: FocusEvent) => void,
onFocusWithinChange?: boolean => void,
onFocusWithinVisibleChange?: boolean => void,
onDetachedVisibleNode?: (e: FocusEvent) => void,
onBeforeActiveElementBlur?: (e: FocusEvent) => void,
onActiveElementBlur?: (e: FocusEvent) => void,
};

type FocusWithinEventType =
| 'focuswithinvisiblechange'
| 'focuswithinchange'
| 'blurwithin'
| 'focuswithin'
| 'detachedvisiblenode';
| 'beforeactiveelementblur'
| 'activeelementblur';

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

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

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',
'activeelementblur',
]
: [
'keydown',
'keyup',
'mousedown',
'touchmove',
'touchstart',
'touchend',
'activeelementblur',
];

function isFunction(obj): boolean {
return typeof obj === 'function';
Expand Down Expand Up @@ -514,18 +532,18 @@ const focusWithinResponderImpl = {
}
break;
}
case 'detachedvisiblenode': {
const onDetachedVisibleNode = (props.onDetachedVisibleNode: any);
if (isFunction(onDetachedVisibleNode)) {
case 'beforeactiveelementblur': {
const onBeforeActiveElementBlur = (props.onBeforeActiveElementBlur: any);
if (isFunction(onBeforeActiveElementBlur)) {
const syntheticEvent = createFocusEvent(
context,
'detachedvisiblenode',
'beforeactiveelementblur',
event.target,
state.pointerType,
);
context.dispatchEvent(
syntheticEvent,
onDetachedVisibleNode,
onBeforeActiveElementBlur,
DiscreteEvent,
);
}
Expand All @@ -538,6 +556,23 @@ const focusWithinResponderImpl = {
props: FocusWithinProps,
state: FocusState,
): void {
if (event.type === 'activeelementblur') {
const onActiveElementBlur = (props.onActiveElementBlur: any);
if (isFunction(onActiveElementBlur)) {
const syntheticEvent = createFocusEvent(
context,
'activeelementblur',
trueadm marked this conversation as resolved.
Show resolved Hide resolved
event.target,
state.pointerType,
);
context.dispatchEvent(
syntheticEvent,
onActiveElementBlur,
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('onBeforeActiveElementBlur/onActiveElementBlur', () => {
let onBeforeActiveElementBlur,
onActiveElementBlur,
ref,
innerRef,
innerRef2;

beforeEach(() => {
onDetachedVisibleNode = jest.fn();
onBeforeActiveElementBlur = jest.fn();
onActiveElementBlur = 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({
onBeforeActiveElementBlur,
onActiveElementBlur,
});
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(onBeforeActiveElementBlur).toHaveBeenCalledTimes(0);
expect(onActiveElementBlur).toHaveBeenCalledTimes(0);
ReactDOM.render(<Component show={false} />, container);
expect(onBeforeActiveElementBlur).toHaveBeenCalledTimes(1);
expect(onActiveElementBlur).toHaveBeenCalledTimes(1);
});

it('is called after a nested focused element is unmounted', () => {
const Component = ({show}) => {
const listener = useFocusWithin({
onBeforeActiveElementBlur,
onActiveElementBlur,
});
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(onBeforeActiveElementBlur).toHaveBeenCalledTimes(0);
expect(onActiveElementBlur).toHaveBeenCalledTimes(0);
ReactDOM.render(<Component show={false} />, container);
expect(onDetachedVisibleNode).toHaveBeenCalledTimes(1);
expect(onBeforeActiveElementBlur).toHaveBeenCalledTimes(1);
expect(onActiveElementBlur).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
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export const mountFundamentalComponent =
export const shouldUpdateFundamentalComponent =
$$$hostConfig.shouldUpdateFundamentalComponent;
export const getInstanceFromNode = $$$hostConfig.getInstanceFromNode;
export const beforeRemoveInstance = $$$hostConfig.beforeRemoveInstance;

// -------------------
// Mutation
Expand Down
4 changes: 4 additions & 0 deletions packages/react-test-renderer/src/ReactTestHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -367,3 +367,7 @@ export function getInstanceFromNode(mockNode: Object) {
}
return null;
}

export function beforeRemoveInstance(instance) {
// noop
}