From cc38c40c3ed9684edc707f1b133ca11dccfa53c9 Mon Sep 17 00:00:00 2001 From: Teng-hao Chang Date: Thu, 21 Mar 2019 18:40:33 +0800 Subject: [PATCH 01/58] =?UTF-8?q?Detect=20=E2=80=9Coutside=20click?= =?UTF-8?q?=E2=80=9D=20in=20closable()=20with=20a=20fixed,=20transparent?= =?UTF-8?q?=20layer=20instead=20of=20listening=20native=20events.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows user to “intercept” click events via React's own mechanism, by stopping the React SyntheticEvent from propagation. Touch event handlers are also removed for similar reasons: Listening only to 'onClick' event help user to intercept. Besides, 'onClick' is eventually simulated on touch devices. --- packages/core/src/mixins/closable.js | 67 +++++++++++++--------------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/packages/core/src/mixins/closable.js b/packages/core/src/mixins/closable.js index a0038d3b..795c72d7 100644 --- a/packages/core/src/mixins/closable.js +++ b/packages/core/src/mixins/closable.js @@ -5,7 +5,14 @@ import keycode from 'keycode'; import getComponentName from '../utils/getComponentName'; const ESCAPE = 'Escape'; -const TOUCH_CLOSE_DELAY = 100; + +const outerLayerStyle = { + position: 'fixed', + width: '100vw', + height: '100vh', + top: 0, + left: 0, +}; /** * `closable()` HOC mixin @@ -51,20 +58,14 @@ const closable = ({ constructor(props) { super(props); this.clickedInside = false; - this.closeDelayTimeout = null; } componentDidMount() { document.addEventListener('keyup', this.handleDocumentKeyup); - document.addEventListener('click', this.handleDocumentClickOrTouch); - document.addEventListener('touchend', this.handleDocumentClickOrTouch); } componentWillUnmount() { document.removeEventListener('keyup', this.handleDocumentKeyup); - document.removeEventListener('click', this.handleDocumentClickOrTouch); - document.removeEventListener('touchend', this.handleDocumentClickOrTouch); - clearTimeout(this.closeDelayTimeout); } getOptions() { @@ -82,24 +83,8 @@ const closable = ({ } /** - * Delay slightly to fire child events first - * before trigger the `onClose` event. + * @param {KeyboardEvent} event */ - delayedClose = (event) => { - this.closeDelayTimeout = setTimeout( - () => this.props.onClose(event), - TOUCH_CLOSE_DELAY, - ); - } - - captureInsideEvents = (node) => { - if (node) { - node.addEventListener('click', this.handleInsideClickOrTouch); - node.addEventListener('touchend', this.handleInsideClickOrTouch); - } - this.nodeRef = node; - } - handleDocumentKeyup = (event) => { const options = this.getOptions(); @@ -108,26 +93,26 @@ const closable = ({ } } - handleDocumentClickOrTouch = (event) => { + /** + * @param {React.MouseEvent} event + */ + handleOuterLayerClick = (event) => { const options = this.getOptions(); - if (this.clickedInside) { - // already scheduled close when clicked inside, skip here. - this.clickedInside = false; - return; - } - if (options.onClickOutside) { - this.delayedClose(event); + this.props.onClose(event); } } - handleInsideClickOrTouch = (event) => { + /** + * @param {React.MouseEvent} event + */ + handleInsideClick = (event) => { const options = this.getOptions(); - this.clickedInside = true; + event.stopPropagation(); if (options.onClickInside) { - this.delayedClose(event); + this.props.onClose(event); } } @@ -140,8 +125,16 @@ const closable = ({ } = this.props; return ( -
- +
+
+ +
); } From a2f20e31ce0e22f308f7ba5b5af866047efb2458 Mon Sep 17 00:00:00 2001 From: Teng-hao Chang Date: Thu, 21 Mar 2019 18:49:11 +0800 Subject: [PATCH 02/58] Move outer layer style to SCSS for closable() --- packages/core/src/mixins/closable.js | 19 +++++++++---------- packages/core/src/styles/Closable.scss | 9 +++++++++ 2 files changed, 18 insertions(+), 10 deletions(-) create mode 100644 packages/core/src/styles/Closable.scss diff --git a/packages/core/src/mixins/closable.js b/packages/core/src/mixins/closable.js index 795c72d7..84cefc12 100644 --- a/packages/core/src/mixins/closable.js +++ b/packages/core/src/mixins/closable.js @@ -2,17 +2,16 @@ import React, { PureComponent } from 'react'; import PropTypes from 'prop-types'; import keycode from 'keycode'; +import icBEM from '../utils/icBEM'; +import prefixClass from '../utils/prefixClass'; import getComponentName from '../utils/getComponentName'; -const ESCAPE = 'Escape'; +import '../styles/Closable.scss'; -const outerLayerStyle = { - position: 'fixed', - width: '100vw', - height: '100vh', - top: 0, - left: 0, -}; +export const COMPONENT_NAME = prefixClass('closable'); +const ROOT_BEM = icBEM(COMPONENT_NAME); + +const ESCAPE = 'Escape'; /** * `closable()` HOC mixin @@ -127,8 +126,8 @@ const closable = ({ return (
+ className={ROOT_BEM.toString()} + onClick={this.handleOuterLayerClick}>
Date: Thu, 21 Mar 2019 18:59:18 +0800 Subject: [PATCH 03/58] =?UTF-8?q?Move=20=E2=80=9Cinside=20click=E2=80=9D?= =?UTF-8?q?=20detection=20to=20component=20that=20uses=20closable()=20HOC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/Popover.js | 19 ++++++++++++++++++- packages/core/src/mixins/closable.js | 10 +++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/core/src/Popover.js b/packages/core/src/Popover.js index 27f6352b..c984ff43 100644 --- a/packages/core/src/Popover.js +++ b/packages/core/src/Popover.js @@ -1,4 +1,5 @@ import React from 'react'; +import PropTypes from 'prop-types'; import classNames from 'classnames'; import ListSpacingContext from './contexts/listSpacing'; @@ -24,10 +25,13 @@ export const BEM = { }; function Popover({ + onClick, // from anchored() placement, arrowStyle, nodeRef, + // from closable() + handleInsideClick, // React props className, children, @@ -36,9 +40,19 @@ function Popover({ const bemClass = BEM.root.modifier(placement); const rootClassName = classNames(bemClass.toString(), className); + const handleWrapperClick = (event) => { + handleInsideClick(event); + onClick(event); + }; + return ( -
+
{children} @@ -49,12 +63,15 @@ function Popover({ } Popover.propTypes = { + onClick: PropTypes.func, placement: anchoredPropTypes.placement, arrowStyle: anchoredPropTypes.arrowStyle, nodeRef: anchoredPropTypes.nodeRef, + handleInsideClick: PropTypes.func.isRequired, }; Popover.defaultProps = { + onClick: () => {}, placement: ANCHORED_PLACEMENT.BOTTOM, arrowStyle: {}, nodeRef: undefined, diff --git a/packages/core/src/mixins/closable.js b/packages/core/src/mixins/closable.js index 84cefc12..5d9fa2c9 100644 --- a/packages/core/src/mixins/closable.js +++ b/packages/core/src/mixins/closable.js @@ -119,7 +119,6 @@ const closable = ({ const { onClose, closable: runtimeOptions, - className, ...otherProps } = this.props; @@ -128,12 +127,9 @@ const closable = ({ role="presentation" className={ROOT_BEM.toString()} onClick={this.handleOuterLayerClick}> -
- -
+
); } From 41ea532abf338a17dff1f5e146c1443595ac9cbc Mon Sep 17 00:00:00 2001 From: Teng-hao Chang Date: Fri, 22 Mar 2019 11:45:15 +0800 Subject: [PATCH 04/58] Update tests for closable() HOC --- .../src/mixins/__tests__/closable.test.js | 90 +++++-------------- 1 file changed, 22 insertions(+), 68 deletions(-) diff --git a/packages/core/src/mixins/__tests__/closable.test.js b/packages/core/src/mixins/__tests__/closable.test.js index da7058e8..d5fd86d1 100644 --- a/packages/core/src/mixins/__tests__/closable.test.js +++ b/packages/core/src/mixins/__tests__/closable.test.js @@ -1,15 +1,25 @@ import React from 'react'; import ReactDOM from 'react-dom'; +import PropTypes from 'prop-types'; + import { mount } from 'enzyme'; import keycode from 'keycode'; -import closable from '../closable'; - -const Foo = () => ( -
Foo
+import closable, { + COMPONENT_NAME as OUTER_LAYER_NAME, +} from '../closable'; + +const Foo = ({ handleInsideClick }) => ( + ); - -jest.useFakeTimers(); +Foo.propTypes = { + handleInsideClick: PropTypes.func.isRequired, +}; it('renders without crashing', () => { const div = document.createElement('div'); @@ -58,19 +68,9 @@ it('tears down event listeners on unmount', () => { const wrapper = mount(); wrapper.unmount(); - let event = new KeyboardEvent('keyup', { keyCode: keycode('Escape') }); - document.dispatchEvent(event); - expect(handleClose).not.toHaveBeenCalled(); - - event = new MouseEvent('click'); + const event = new KeyboardEvent('keyup', { keyCode: keycode('Escape') }); document.dispatchEvent(event); expect(handleClose).not.toHaveBeenCalled(); - - event = new CustomEvent('touchstart'); - document.dispatchEvent(event); - - jest.runAllTimers(); - expect(handleClose).not.toHaveBeenCalled(); }); describe.each` @@ -98,7 +98,7 @@ describe.each` const handleClose = jest.fn(); const closableOptions = { onClickInside, onClickOutside }; - const wrapper = mount( + mount( , { attachTo: rootNode }, ); @@ -112,8 +112,6 @@ describe.each` keyEvent = new KeyboardEvent('keyup', { keyCode: keycode('Escape') }); document.dispatchEvent(keyEvent); expect(handleClose).toHaveBeenCalledTimes(shouldBeCalled ? 1 : 0); - - wrapper.unmount(); }); }); @@ -124,15 +122,6 @@ describe.each` ${false} | ${false} | ${'should not'} `('$desc call onClose() for inside-clicks when onClickInside=$onClickInside', ({ onClickInside, shouldBeCalled }) => { const ClosableFoo = closable({ onClickInside })(Foo); - const rootNode = document.createElement('div'); - - beforeAll(() => { - document.body.appendChild(rootNode); - }); - - afterAll(() => { - document.body.removeChild(rootNode); - }); it.each([ { onEscape: true, onClickOutside: true }, @@ -144,24 +133,11 @@ describe.each` const closableOptions = { onEscape, onClickOutside }; const wrapper = mount( - , - { attachTo: rootNode }, + ); - let event = new MouseEvent('click', { bubbles: true }); - wrapper.instance().nodeRef.dispatchEvent(event); - - jest.runOnlyPendingTimers(); + wrapper.find('div#foo').simulate('click'); expect(handleClose).toHaveBeenCalledTimes(shouldBeCalled ? 1 : 0); - - // jsdom doesn't support constructing TouchEvent yet. - event = new CustomEvent('touchend', { bubbles: true }); - wrapper.instance().nodeRef.dispatchEvent(event); - - jest.runOnlyPendingTimers(); - expect(handleClose).toHaveBeenCalledTimes(shouldBeCalled ? 2 : 0); - - wrapper.unmount(); }); }); @@ -171,15 +147,6 @@ describe.each` ${false} | ${false} | ${'should not'} `('$desc call onClose() when onClickOutside=$onClickOutside', ({ onClickOutside, shouldBeCalled }) => { const ClosableFoo = closable({ onClickOutside })(Foo); - const rootNode = document.createElement('div'); - - beforeAll(() => { - document.body.appendChild(rootNode); - }); - - afterAll(() => { - document.body.removeChild(rootNode); - }); it.each([ { onEscape: true, onClickInside: true }, @@ -191,23 +158,10 @@ describe.each` const closableOptions = { onEscape, onClickInside }; const wrapper = mount( - , - { attachTo: rootNode }, + ); - let event = new MouseEvent('click'); - document.dispatchEvent(event); - - jest.runOnlyPendingTimers(); + wrapper.find(`.${OUTER_LAYER_NAME}`).simulate('click'); expect(handleClose).toHaveBeenCalledTimes(shouldBeCalled ? 1 : 0); - - // jsdom doesn't support constructing TouchEvent yet. - event = new CustomEvent('touchend'); - document.dispatchEvent(event); - - jest.runOnlyPendingTimers(); - expect(handleClose).toHaveBeenCalledTimes(shouldBeCalled ? 2 : 0); - - wrapper.unmount(); }); }); From 1ddf2bd3d6c569830edd9711787a4e726dd8298c Mon Sep 17 00:00:00 2001 From: Teng-hao Chang Date: Fri, 22 Mar 2019 11:52:36 +0800 Subject: [PATCH 05/58] Update CHANGELOG about closable() refactor --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bef1a1d1..274c8754 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Changed +- [Core] Refactored `closable()` mixin to detect inside/outside clicks via React SyntheticEvent mechanism instead of listening native events from DOM. ## [2.1.0] ### Changed From 7d0c67bbdce8b378b3001fa3cad78c5341776ea8 Mon Sep 17 00:00:00 2001 From: tz5514 Date: Mon, 25 Mar 2019 14:09:49 +0800 Subject: [PATCH 06/58] setup react-textarea-autosize --- packages/form/package.json | 3 ++- yarn.lock | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/form/package.json b/packages/form/package.json index 83d8f83a..32450346 100644 --- a/packages/form/package.json +++ b/packages/form/package.json @@ -37,7 +37,8 @@ "@ichef/gypcrete": "^2.1.0", "classnames": "^2.2.5", "immutable": "^3.8.2", - "keycode": "^2.1.9" + "keycode": "^2.1.9", + "react-textarea-autosize": "^7.1.0" }, "devDependencies": { "@babel/cli": "^7.1.0", diff --git a/yarn.lock b/yarn.lock index 64a4bb39..4042a666 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9974,6 +9974,11 @@ react-is@^16.6.1: resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.6.1.tgz#f77b1c3d901be300abe8d58645b7a59e794e5982" integrity sha512-wOKsGtvTMYs7WAscmwwdM8sfRRvE17Ym30zFj3n37Qx5tHRfhenPKEPILHaHob6WoLFADmQm1ZNrE5xMCM6sCw== +react-is@^16.8.5: + version "16.8.5" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.8.5.tgz#c54ac229dd66b5afe0de5acbe47647c3da692ff8" + integrity sha512-sudt2uq5P/2TznPV4Wtdi+Lnq3yaYW8LfvPKLM9BKD8jJNBkxMVyB0C9/GmVhLw7Jbdmndk/73n7XQGeN9A3QQ== + react-lifecycles-compat@^3.0.0, react-lifecycles-compat@^3.0.4: version "3.0.4" resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362" @@ -10006,7 +10011,7 @@ react-style-proptype@^3.0.0: dependencies: prop-types "^15.5.4" -react-test-renderer@^16.0.0-0, react-test-renderer@^16.6.1: +react-test-renderer@^16.0.0-0: version "16.6.1" resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-16.6.1.tgz#8ea357652be3cf81cbd6b2f686e74ebe67c17b78" integrity sha512-sgZwJZYIgQptRi2qk5+gB8FBQGk4gLSs0gmKZPMfhd3dLkdxIUwVLHteLN3Bnj4LokIZd3U+V2NEJUqeV2PT2w== @@ -10016,6 +10021,16 @@ react-test-renderer@^16.0.0-0, react-test-renderer@^16.6.1: react-is "^16.6.1" scheduler "^0.11.0" +react-test-renderer@^16.6.1: + version "16.8.5" + resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-16.8.5.tgz#4cba7a8aad73f7e8a0bc4379a0fe21632886a563" + integrity sha512-/pFpHYQH4f35OqOae/DgOCXJDxBqD3K3akVfDhLgR0qYHoHjnICI/XS9QDwIhbrOFHWL7okVW9kKMaHuKvt2ng== + dependencies: + object-assign "^4.1.1" + prop-types "^15.6.2" + react-is "^16.8.5" + scheduler "^0.13.5" + react-textarea-autosize@^7.0.4: version "7.0.4" resolved "https://registry.yarnpkg.com/react-textarea-autosize/-/react-textarea-autosize-7.0.4.tgz#4e4be649b544a88713e7b5043f76950f35d3d503" @@ -10023,6 +10038,14 @@ react-textarea-autosize@^7.0.4: dependencies: prop-types "^15.6.0" +react-textarea-autosize@^7.1.0: + version "7.1.0" + resolved "https://registry.yarnpkg.com/react-textarea-autosize/-/react-textarea-autosize-7.1.0.tgz#3132cb77e65d94417558d37c0bfe415a5afd3445" + integrity sha512-c2FlR/fP0qbxmlrW96SdrbgP/v0XZMTupqB90zybvmDVDutytUgPl7beU35klwcTeMepUIQEpQUn3P3bdshGPg== + dependencies: + "@babel/runtime" "^7.1.2" + prop-types "^15.6.0" + react-transition-group@^2.0.0: version "2.5.0" resolved "https://registry.yarnpkg.com/react-transition-group/-/react-transition-group-2.5.0.tgz#70bca0e3546102c4dc5cf3f5f57f73447cce6874" @@ -10761,6 +10784,14 @@ scheduler@^0.11.2: loose-envify "^1.1.0" object-assign "^4.1.1" +scheduler@^0.13.5: + version "0.13.5" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.13.5.tgz#b7226625167041298af3b98088a9dbbf6d7733a8" + integrity sha512-K98vjkQX9OIt/riLhp6F+XtDPtMQhqNcf045vsh+pcuvHq+PHy1xCrH3pq1P40m6yR46lpVvVhKdEOtnimuUJw== + dependencies: + loose-envify "^1.1.0" + object-assign "^4.1.1" + schema-utils@^0.3.0: version "0.3.0" resolved "https://registry.yarnpkg.com/schema-utils/-/schema-utils-0.3.0.tgz#f5877222ce3e931edae039f17eb3716e7137f8cf" From 1459ad51134f21d5700e93b434d3919455284663 Mon Sep 17 00:00:00 2001 From: tz5514 Date: Mon, 25 Mar 2019 15:13:16 +0800 Subject: [PATCH 07/58] refactor TextInputRow for custom input & auto size textarea --- packages/form/src/TextInputRow.js | 112 +++++++++++------------------- 1 file changed, 42 insertions(+), 70 deletions(-) diff --git a/packages/form/src/TextInputRow.js b/packages/form/src/TextInputRow.js index 174beba0..510a5cbd 100644 --- a/packages/form/src/TextInputRow.js +++ b/packages/form/src/TextInputRow.js @@ -1,11 +1,10 @@ +/* eslint-disable class-methods-use-this */ import React, { PureComponent } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; -import { - ListRow, - TextLabel, -} from '@ichef/gypcrete'; +import { ListRow, TextLabel } from '@ichef/gypcrete'; +import AutoSizeTextarea from 'react-textarea-autosize'; import prefixClass from '@ichef/gypcrete/lib/utils/prefixClass'; import icBEM from '@ichef/gypcrete/lib/utils/icBEM'; @@ -21,10 +20,16 @@ export const BEM = { input: ROOT_BEM.element('input'), }; +const isFunction = func => (typeof func === 'function'); + class TextInputRow extends PureComponent { static propTypes = { label: PropTypes.node.isRequired, multiLine: PropTypes.bool, + minRows: PropTypes.number, + maxRows: PropTypes.number, + renderInput: PropTypes.func, + inputComponent: PropTypes.func, // input props value: PropTypes.string, placeholder: PropTypes.string, @@ -38,6 +43,10 @@ class TextInputRow extends PureComponent { static defaultProps = { multiLine: false, + minRows: 2, + maxRows: undefined, + renderInput: undefined, + inputComponent: undefined, value: undefined, placeholder: 'Unset', onChange: () => {}, @@ -49,44 +58,10 @@ class TextInputRow extends PureComponent { state = { focused: false, - inputHeight: 'auto', }; - componentDidMount() { - this.updateInputHeight(this.getInputRef()); - } - - componentDidUpdate(prevProps) { - if (this.props.value !== prevProps.value) { - this.updateInputHeight(this.getInputRef()); - } - } - - setInputRef = (ref) => { - this.inputRef = ref; - } - - getInputRef = () => this.inputRef; - - /** - * Use `getInputRef()` instead. - * Should remove in v2. - * @deprecated - */ - getInputNode = this.getInputRef; - - updateInputHeight(inputNode) { - const newHeight = inputNode.scrollHeight; - this.setState({ inputHeight: newHeight }); - } - handleInputFocus = (event) => { - const inputNode = event.target; - - this.setState( - { focused: true }, - () => this.updateInputHeight(inputNode) - ); + this.setState({ focused: true }); this.props.onFocus(event); } @@ -95,55 +70,51 @@ class TextInputRow extends PureComponent { this.props.onBlur(event); } - handleInputChange = (event) => { - this.updateInputHeight(event.target); - this.props.onChange(event); - } - renderInput({ multiLine, - style: inputStyle = {}, - ...inputProps + style = {}, + renderInput, + inputComponent, + minRows, + maxRows, + ...otherProps }) { const sharedProps = { - ref: this.setInputRef, className: BEM.input.toString(), + style, onFocus: this.handleInputFocus, onBlur: this.handleInputBlur, + ...otherProps }; - const textareaStyle = { - ...inputStyle, - height: this.state.inputHeight, + if (multiLine) { + const textareaProps = { + ...sharedProps, + minRows, + maxRows, + }; + + return ; + } + + const inputProps = { + type: 'text', + ...sharedProps, }; - if (multiLine) { - return ( -