Skip to content

Commit

Permalink
Remove native key handling (iTwin#2545)
Browse files Browse the repository at this point in the history
* Remove native key handling in React editor components.

* Rush change

* extract-api
  • Loading branch information
NancyMcCallB authored Oct 20, 2021
1 parent ce0b24e commit 47a81bf
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 38 deletions.
6 changes: 1 addition & 5 deletions common/api/components-react.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -891,13 +891,9 @@ export interface EditableTreeDataProvider extends ITreeDataProvider {

// @public
export class EditorContainer extends React.PureComponent<EditorContainerProps> {
// (undocumented)
componentDidMount(): void;
// (undocumented)
componentWillUnmount(): void;
// @internal (undocumented)
render(): JSX.Element;
}
}

// @public
export interface EditorContainerProps extends CommonProps {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/components-react",
"comment": "Remove native key handling in React editor components because it's not longer needed with React 17.",
"type": "none"
}
],
"packageName": "@itwin/components-react"
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export class CustomNumberEditor extends React.PureComponent<PropertyEditorProps,
this.setState({ inputValue: initialDisplayValue });
}

private _onKeyPress = (e: KeyboardEvent) => {
private _onKeyPress = (e: React.KeyboardEvent<HTMLInputElement>) => {
// istanbul ignore else
if (e.key === SpecialKey.Escape) {
const initialDisplayValue = this._getInitialDisplayValue();
Expand Down Expand Up @@ -272,7 +272,7 @@ export class CustomNumberEditor extends React.PureComponent<PropertyEditorProps,
onBlur: this.props.onBlur,
onFocus: this._onFocus,
setFocus: this.shouldSetFocus(),
nativeKeyHandler: this._onKeyPress,
onKeyDown: this._onKeyPress,
};

let reactNode: React.ReactNode;
Expand Down
4 changes: 2 additions & 2 deletions ui/components/src/components-react/editors/DateTimeEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ export class DateTimeEditor extends React.PureComponent<DateTimeEditorProps, Dat
public get hasFocus(): boolean {
let containsFocus = false;
// istanbul ignore else
if (this._divElement.current)
containsFocus = this._divElement.current.contains(document.activeElement);
if (this.htmlElement)
containsFocus = this.htmlElement.contains(document.activeElement);
return containsFocus;
}

Expand Down
19 changes: 5 additions & 14 deletions ui/components/src/components-react/editors/EditorContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ export class EditorContainer extends React.PureComponent<EditorContainerProps> {

private _editorRef: TypeEditor | undefined;
private _propertyEditor: PropertyEditorBase | undefined;
private _spanRef = React.createRef<HTMLSpanElement>();

private createEditor(): React.ReactNode {
const editorRef = (ref: TypeEditor | undefined) => this._editorRef = ref;
Expand Down Expand Up @@ -109,14 +108,6 @@ export class EditorContainer extends React.PureComponent<EditorContainerProps> {

return clonedNode;
}
public override componentDidMount() {
this._spanRef.current?.addEventListener("keydown", this._handleKeyDown, true);
}
public override componentWillUnmount() {
// istanbul ignore next
if (this._spanRef.current)
this._spanRef.current.removeEventListener("keydown", this._handleKeyDown, true);
}

private _handleEditorBlur = (_e: React.FocusEvent) => {
// istanbul ignore else
Expand All @@ -132,7 +123,7 @@ export class EditorContainer extends React.PureComponent<EditorContainerProps> {
e.stopPropagation();
};

private _handleKeyDown = (e: KeyboardEvent) => {
private _handleKeyDown = (e: React.KeyboardEvent) => {
switch (e.key) {
case SpecialKey.Escape:
this.onPressEscape(e);
Expand All @@ -153,14 +144,14 @@ export class EditorContainer extends React.PureComponent<EditorContainerProps> {
e.stopPropagation();
};

private onPressEscape(_e: KeyboardEvent): void {
private onPressEscape(_e: React.KeyboardEvent): void {
// istanbul ignore else
if (this._propertyEditor && this._propertyEditor.containerHandlesEscape) {
this._commitCancel();
}
}

private onPressEnter(e: KeyboardEvent): void {
private onPressEnter(e: React.KeyboardEvent): void {
// istanbul ignore else
if (this._propertyEditor && this._propertyEditor.containerHandlesEnter) {
// istanbul ignore else
Expand All @@ -170,7 +161,7 @@ export class EditorContainer extends React.PureComponent<EditorContainerProps> {
}
}

private onPressTab(e: KeyboardEvent): void {
private onPressTab(e: React.KeyboardEvent): void {
// istanbul ignore else
if (this._propertyEditor && this._propertyEditor.containerHandlesTab) {
e.stopPropagation();
Expand Down Expand Up @@ -253,12 +244,12 @@ export class EditorContainer extends React.PureComponent<EditorContainerProps> {
return (
<span className="components-editor-container"
onBlur={this._handleContainerBlur}
onKeyDown={this._handleKeyDown}
onClick={this._handleClick}
onContextMenu={this._handleRightClick}
title={this.props.title}
data-testid="editor-container"
role="presentation"
ref={this._spanRef}
>
{this.createEditor()}
</span>
Expand Down
15 changes: 2 additions & 13 deletions ui/components/src/components-react/editors/PopupButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,6 @@ export class PopupButton extends React.PureComponent<PopupButtonProps, PopupButt
public override componentDidMount() {
if (this.props.setFocus && this._buttonRef.current)
this._buttonRef.current.focus();
this._buttonRef.current?.addEventListener("keydown", this._handleKeyDown);
}

/** @internal */
public override componentWillUnmount() {
// istanbul ignore next
if (this._buttonRef.current)
this._buttonRef.current.removeEventListener("keydown", this._handleKeyDown);
}

private _togglePopup = (event: React.MouseEvent) => {
Expand All @@ -100,10 +92,7 @@ export class PopupButton extends React.PureComponent<PopupButtonProps, PopupButt
});
};

private _emptyKeyDown = (_event: React.KeyboardEvent) => {

};
private _handleKeyDown = (event: KeyboardEvent) => {
private _handleKeyDown = (event: React.KeyboardEvent) => {
// istanbul ignore else
if ((event.key === SpecialKey.ArrowDown || event.key === SpecialKey.Space || event.key === SpecialKey.Enter) && !this.state.showPopup) {
event.preventDefault();
Expand Down Expand Up @@ -133,7 +122,7 @@ export class PopupButton extends React.PureComponent<PopupButtonProps, PopupButt
<div className={this.props.className}>
<div className={classNames}
onClick={this._togglePopup}
onKeyDown={this._emptyKeyDown}
onKeyDown={this._handleKeyDown}
data-testid="components-popup-button"
tabIndex={0}
ref={this._buttonRef}
Expand Down
4 changes: 2 additions & 2 deletions ui/components/src/components-react/editors/SliderEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ export class SliderEditor extends React.PureComponent<PropertyEditorProps, Slide
public get hasFocus(): boolean {
let containsFocus = false;
// istanbul ignore else
if (this._divElement.current)
containsFocus = this._divElement.current.contains(document.activeElement);
if (this.htmlElement)
containsFocus = this.htmlElement.contains(document.activeElement);
return containsFocus;
}

Expand Down
95 changes: 95 additions & 0 deletions ui/components/src/test/ReactInstance.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
/*---------------------------------------------------------------------------------------------
* Source file from https://github.com/arqex/react-dom-instance, updated to work with React 17
* https://www.npmjs.com/package/react-dom-instance
*--------------------------------------------------------------------------------------------*/
const optionsDefault = {
maxIteration: 4,
};
/** For testing only
* @internal
*/
export function findInstance(node: any, opts?: any): any {
const options = Object.assign({}, optionsDefault, opts);

const fiber = getFiberFromNode(node);
if (!fiber) return false;

const instance = getInstanceFromFiber(fiber, options.maxIteration);
if (!instance) return false;

const target = getTargetInstance(instance, instance, options.componentName, options.maxIteration);
return target;
}

function getFiberFromNode(node: any) {
let key = getFiberKey(node);

if (!key) {
node = node.children[0];
key = node && getFiberKey(node);
}

return key ? node[key] : false;
}

function getFiberKey(node: any) {
return Object.keys(node).find((key) => (
key.startsWith("__reactFiber$")
));
}

function getInstanceFromFiber(fiber: any, i: number) {
let f = fiber;

// return isInstanceFiber(f) ? f.stateNode : false;

while (!isInstanceFiber(f) && i-- > 0) {
f = f && f.return;
}

return isInstanceFiber(f) ? f.stateNode : false;
}

function isInstanceFiber(fiber: any) {
return fiber && fiber.type && typeof fiber.type !== "string" && fiber.stateNode;
}
function getTargetInstance(childInstance: any, parentInstance: any, componentName: any, i: number): any {
// console.log('getting instance from fiber', childInstance, parentInstance);
if (!childInstance && !parentInstance) return false;

if (childInstance && isTarget(childInstance, componentName)) {
return childInstance;
}
if (parentInstance && childInstance !== parentInstance && isTarget(parentInstance, componentName)) {
return parentInstance;
}

if (i <= 0) {
warn("maxIteration exceeded and not" + componentName + " instance found."); // eslint-disable-line prefer-template
return false;
}

const childFiber = childInstance && childInstance._reactInternalFiber.child;
const parentFiber = parentInstance && parentInstance._reactInternalFiber.return;

return getTargetInstance(
isInstanceFiber(childFiber) && childFiber.stateNode,
isInstanceFiber(parentFiber) && parentFiber.stateNode,
componentName,
i - 1
);
}

function isTarget(instance: any, componentName: any) {
if (!componentName) return true;

return instance.constructor.name === componentName;
}

function warn(msg: string) {
typeof console !== undefined && console.warn("ReactInstance:" + msg); // eslint-disable-line prefer-template, no-console
}
12 changes: 12 additions & 0 deletions ui/components/src/test/editors/DateTimeEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { EditorContainer /* PropertyUpdatedArgs */ } from "../../components-reac
import { DateTimeEditor } from "../../components-react/editors/DateTimeEditor";
import TestUtils, { MineDataController } from "../TestUtils";
import { PropertyEditorManager } from "../../components-react/editors/PropertyEditorManager";
import { findInstance } from "../ReactInstance";

function createDateProperty(propertyName: string, value: Date, option: number) {
const v: PropertyValue = {
Expand Down Expand Up @@ -250,4 +251,15 @@ describe("<DateTimeEditor />", () => {
PropertyEditorManager.deregisterDataController("myData");
});

it("should receive focus", async () => {
const record = createDateProperty("Test", date, 0); // 0 creates a long DateTime record
const renderedComponent = render(<DateTimeEditor showTime={true} propertyRecord={record} />);
expect(await waitFor(() => renderedComponent.getByText(date.toLocaleString()))).to.exist;
const popupButton = await renderedComponent.findByTestId("components-popup-button");
expect(popupButton).not.to.be.null;
popupButton.focus();
const editor = findInstance(renderedComponent.container.firstChild);
expect (editor).not.to.be.null;
expect (editor.hasFocus).to.be.true;
});
});
12 changes: 12 additions & 0 deletions ui/components/src/test/editors/SliderEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { SliderEditor } from "../../components-react/editors/SliderEditor";
import TestUtils, { MineDataController } from "../TestUtils";
import { EditorContainer } from "../../components-react/editors/EditorContainer";
import { PropertyEditorManager } from "../../components-react/editors/PropertyEditorManager";
import { findInstance } from "../ReactInstance";

describe("<SliderEditor />", () => {
before(async () => {
Expand Down Expand Up @@ -641,5 +642,16 @@ describe("<SliderEditor />", () => {

PropertyEditorManager.deregisterDataController("myData");
});
it("should receive focus", async () => {
const propertyRecord = TestUtils.createNumericProperty("Test", 50, StandardEditorNames.Slider);
const renderedComponent = render(<SliderEditor propertyRecord={propertyRecord} />);
expect(renderedComponent).not.to.be.undefined;
const popupButton = await renderedComponent.findByTestId("components-popup-button");
expect(popupButton).not.to.be.null;
popupButton.focus();
const editor = findInstance(renderedComponent.container.firstChild);
expect (editor).not.to.be.null;
expect (editor.hasFocus).to.be.true;
});

});

0 comments on commit 47a81bf

Please sign in to comment.