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

Fix click outside logic #1417

Merged
merged 19 commits into from
Feb 11, 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
89 changes: 89 additions & 0 deletions packages/common/editors/ClickOutside.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import React from 'react';
import PropTypes from 'prop-types';

/**
* Detecting outside click on a react component is surprisingly hard.
* A general approach is to have a global click handler on the document
* which checks if the click target is inside the editor container or
* not using editorContainer.contains(e.target). This approach works well
* until portals are used for editors. Portals render children into a DOM
* node that exists outside the DOM hierarchy of the parent component so
* editorContainer.contains(e.target) does not work. Here are some examples
* of the DOM structure with different types of editors
*
*
* SimpleEditor for example Texbox (No Portals)
* <div react-data-grid>..<div>
* <div portal-created-by-the-grid-for-editors>
* <div editor-container>
* <div simple-editor>..</div>
* </div>
* <div>
*
* ComplexEditor for example Modals (using Portals)
* <div react-data-grid>..<div>
* <div portal-created-by-the-grid-for-editors>
* <div editor-container>
* // Nothing here
* </div>
* <div>
* <div portal-created-by-the-editor>
* <div complex-editor>..</div>
* </div>
*
*
* One approach to detect outside click is to use event bubbling through
* portals. An event fired from inside a portal will propagate to ancestors
* in the containing React tree, even if those elements are not ancestors
* in the DOM tree. This means a click handler can be attached on the document
* and on the editor container. The editor container can set a flag to notify
* that the click was inside the editor and the document click handler can use
* this flag to call onClickOutside. This approach however has a few caveats
* - Click handler on the document is set using document.addEventListener
* - Click handler on the editor container is set using onClick prop
*
* This means if a child component inside the editor calls e.stopPropagation
* then the click handler on the editor container will not be called whereas
* document click handler will be called.
* https://github.com/facebook/react/issues/12518
*
* To solve this issue onClickCapture event is used.
*/

export default class ClickOutside extends React.Component {
static propTypes = {
children: PropTypes.element.isRequired,
onClickOutside: PropTypes.func.isRequired
};

isClickedInside = false;

componentDidMount() {
document.addEventListener('click', this.handleDocumentClick);
}

componentWillUnmount() {
document.removeEventListener('click', this.handleDocumentClick);
}

handleDocumentClick = (e) => {
if (this.isClickedInside) {
this.isClickedInside = false;
return;
}

this.props.onClickOutside(e);
};

handleClick = () => {
this.isClickedInside = true;
};

render() {
return React.cloneElement(
React.Children.only(this.props.children), {
onClickCapture: this.handleClick
}
);
}
}
42 changes: 7 additions & 35 deletions packages/common/editors/EditorContainer.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import React from 'react';
import ReactDOM from 'react-dom';

import PropTypes from 'prop-types';
import joinClasses from 'classnames';

import SimpleTextEditor from './SimpleTextEditor';
import { isFunction } from 'common/utils';
import { isKeyPrintable, isCtrlKeyHeldDown } from 'common/utils/keyboardUtils';
import * as zIndexes from 'common/constants/zIndexes';
import EditorPortal from './EditorPortal';
import ClickOutside from './ClickOutside';

require('../../../themes/react-data-grid-core.css');

Expand Down Expand Up @@ -36,7 +35,6 @@ class EditorContainer extends React.Component {
changeCanceled = false;

componentDidMount() {
document.addEventListener('click', this.handleDocumentClick);
const inputNode = this.getInputNode();
if (inputNode !== undefined) {
this.setTextInputFocus();
Expand All @@ -54,33 +52,11 @@ class EditorContainer extends React.Component {
}

componentWillUnmount() {
document.removeEventListener('click', this.handleDocumentClick);
if (!this.changeCommitted && !this.changeCanceled) {
this.commit({ key: 'Enter' });
}
}

handleDocumentClick = (e) => {
const { target } = e;
const { container, editor } = this;
if (container && (target === container || container.contains(target))) {
// Clicked inside the editor container
return;
}

if (editor) {
// Check the editor node in case the editor is using a Portal
const editorNode = ReactDOM.findDOMNode(editor);
if (editorNode && (target === editorNode || editorNode.contains(target))) {
// Clicked inside the editor
return;
}
}

// Clicked outside the editor, commit changes
this.commit(e);
};

isKeyExplicitlyHandled = (key) => {
return isFunction(this['onPress' + key]);
};
Expand Down Expand Up @@ -115,10 +91,6 @@ class EditorContainer extends React.Component {
this.editor = editor;
}

setContainerRef = (container) => {
this.container = container;
}

createEditor = () => {
const editorProps = {
ref: this.setEditorRef,
Expand Down Expand Up @@ -332,20 +304,20 @@ class EditorContainer extends React.Component {
};

render() {
const { width, height, left, top, column } = this.props;
const { width, height, left, top } = this.props;
const style = { position: 'absolute', height, width, left, top, zIndex: zIndexes.EDITOR_CONTAINER };
return (
<EditorPortal>
<div style={style}
ref={this.setContainerRef}
<ClickOutside onClickOutside={this.commit}>
<div
style={style}
className={this.getContainerClass()}
onKeyDown={this.onKeyDown}
onContextMenu={this.handleRightClick}
>
{this.createEditor()}
{this.renderStatusIcon()}
</div>
</EditorPortal>
</ClickOutside>
);
}
}
Expand Down
22 changes: 17 additions & 5 deletions packages/common/editors/EditorPortal.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,36 @@ import React from 'react';
import ReactDOM from 'react-dom';
import PropTypes from 'prop-types';

const editorRoot = document.body;

export default class EditorPortal extends React.Component {
static propTypes = {
children: PropTypes.node
children: PropTypes.node.isRequired,
target: PropTypes.instanceOf(Element).isRequired
};

// Keep track of when the modal element is added to the DOM
state = {
isMounted: false
};

el = document.createElement('div');

componentDidMount() {
editorRoot.appendChild(this.el);
this.props.target.appendChild(this.el);
// eslint-disable-next-line react/no-did-mount-set-state
this.setState({ isMounted: true });
}

componentWillUnmount() {
editorRoot.removeChild(this.el);
this.props.target.removeChild(this.el);
}

render() {
// Don't render the portal until the component has mounted,
Copy link
Contributor Author

@amanmahajan7 amanmahajan7 Feb 11, 2019

Choose a reason for hiding this comment

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

This is needed for editors that require the editor container to be attached to DOM during componentDidMount. Legacy editors uses jquery to initialize the component in componentDidMount and it breaks if the element is not attached to body

// So the portal can safely access the DOM.
if (!this.state.isMounted) {
return null;
}

return ReactDOM.createPortal(
this.props.children,
this.el,
Expand Down
Loading