From 1b02695fc428be14679d38f1db0236e397e2e20b Mon Sep 17 00:00:00 2001 From: Fox Date: Thu, 21 Sep 2017 15:38:02 -0700 Subject: [PATCH 1/4] Fix for Portal usage of EventStack sub/unsub --- src/addons/Portal/Portal.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/addons/Portal/Portal.js b/src/addons/Portal/Portal.js index f983367933..162cf1c467 100644 --- a/src/addons/Portal/Portal.js +++ b/src/addons/Portal/Portal.js @@ -139,6 +139,7 @@ class Portal extends Component { componentDidMount() { debug('componentDidMount()') + this.uniqueId = _.uniqueId() this.renderPortal() } @@ -384,8 +385,8 @@ class Portal extends Component { mountNode.appendChild(this.rootNode) } - eventStack.sub('click', this.handleDocumentClick, 'Portal') - eventStack.sub('keydown', this.handleEscape, 'Portal') + eventStack.sub('click', this.handleDocumentClick, `Portal${this.uniqueId}`) + eventStack.sub('keydown', this.handleEscape, `Portal${this.uniqueId}`) _.invoke(this.props, 'onMount', null, this.props) } @@ -403,8 +404,8 @@ class Portal extends Component { this.rootNode = null this.portalNode = null - eventStack.unsub('click', this.handleDocumentClick, 'Portal') - eventStack.unsub('keydown', this.handleEscape, 'Portal') + eventStack.unsub('click', this.handleDocumentClick, `Portal${this.uniqueId}`) + eventStack.unsub('keydown', this.handleEscape, `Portal${this.uniqueId}`) _.invoke(this.props, 'onUnmount', null, this.props) } From 24ee33cf98dc778dacd9d5da56eb7e4a4fa29479 Mon Sep 17 00:00:00 2001 From: Fox Date: Fri, 22 Sep 2017 08:53:37 -0700 Subject: [PATCH 2/4] Responding to PR comments --- src/addons/Portal/Portal.d.ts | 3 +++ src/addons/Portal/Portal.js | 16 +++++++++++----- src/modules/Modal/Modal.d.ts | 3 +++ src/modules/Modal/Modal.js | 7 ++++++- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/addons/Portal/Portal.d.ts b/src/addons/Portal/Portal.d.ts index e8f742cce2..87c840c129 100644 --- a/src/addons/Portal/Portal.d.ts +++ b/src/addons/Portal/Portal.d.ts @@ -95,6 +95,9 @@ export interface PortalProps { /** Controls whether or not the portal should open when mousing over the trigger. */ openOnTriggerMouseEnter?: boolean; + /** Event pool namespace that is used to handle component events. */ + eventPool?: string; + /** Controls whether the portal should be prepended to the mountNode instead of appended. */ prepend?: boolean; diff --git a/src/addons/Portal/Portal.js b/src/addons/Portal/Portal.js index 162cf1c467..ae0da43f2c 100644 --- a/src/addons/Portal/Portal.js +++ b/src/addons/Portal/Portal.js @@ -115,6 +115,9 @@ class Portal extends Component { /** Controls whether or not the portal should open when mousing over the trigger. */ openOnTriggerMouseEnter: PropTypes.bool, + /** Event pool namespace that is used to handle component events */ + eventPool: PropTypes.string, + /** Controls whether the portal should be prepended to the mountNode instead of appended. */ prepend: PropTypes.bool, @@ -126,6 +129,7 @@ class Portal extends Component { closeOnDocumentClick: true, closeOnEscape: true, openOnTriggerClick: true, + eventPool: 'default' } static autoControlledProps = [ @@ -139,7 +143,6 @@ class Portal extends Component { componentDidMount() { debug('componentDidMount()') - this.uniqueId = _.uniqueId() this.renderPortal() } @@ -375,6 +378,7 @@ class Portal extends Component { const { mountNode = isBrowser ? document.body : null, prepend, + eventPool } = this.props this.rootNode = document.createElement('div') @@ -385,8 +389,8 @@ class Portal extends Component { mountNode.appendChild(this.rootNode) } - eventStack.sub('click', this.handleDocumentClick, `Portal${this.uniqueId}`) - eventStack.sub('keydown', this.handleEscape, `Portal${this.uniqueId}`) + eventStack.sub('click', this.handleDocumentClick, eventPool) + eventStack.sub('keydown', this.handleEscape, eventPool) _.invoke(this.props, 'onMount', null, this.props) } @@ -395,6 +399,8 @@ class Portal extends Component { debug('unmountPortal()') + const { eventPool } = this.props + ReactDOM.unmountComponentAtNode(this.rootNode) this.rootNode.parentNode.removeChild(this.rootNode) @@ -404,8 +410,8 @@ class Portal extends Component { this.rootNode = null this.portalNode = null - eventStack.unsub('click', this.handleDocumentClick, `Portal${this.uniqueId}`) - eventStack.unsub('keydown', this.handleEscape, `Portal${this.uniqueId}`) + eventStack.unsub('click', this.handleDocumentClick, eventPool) + eventStack.unsub('keydown', this.handleEscape, eventPool) _.invoke(this.props, 'onUnmount', null, this.props) } diff --git a/src/modules/Modal/Modal.d.ts b/src/modules/Modal/Modal.d.ts index dcb5d0afab..e6775fba52 100644 --- a/src/modules/Modal/Modal.d.ts +++ b/src/modules/Modal/Modal.d.ts @@ -34,6 +34,9 @@ export interface ModalProps extends PortalProps { /** Whether or not the Modal should close when the document is clicked. */ closeOnDocumentClick?: boolean; + /** Event pool namespace that is used to handle component events */ + eventPool?: string; + /** A Modal can be passed content via shorthand. */ content?: SemanticShorthandItem; diff --git a/src/modules/Modal/Modal.js b/src/modules/Modal/Modal.js index d8cf253360..bc62f2b229 100644 --- a/src/modules/Modal/Modal.js +++ b/src/modules/Modal/Modal.js @@ -58,6 +58,9 @@ class Modal extends Component { /** Whether or not the Modal should close when the document is clicked. */ closeOnDocumentClick: PropTypes.bool, + /** Event pool namespace that is used to handle component events */ + eventPool: PropTypes.string, + /** Simple text content for the Modal. */ content: customPropTypes.itemShorthand, @@ -135,6 +138,7 @@ class Modal extends Component { dimmer: true, closeOnDimmerClick: true, closeOnDocumentClick: false, + eventPool: 'Modal' } static autoControlledProps = [ @@ -315,7 +319,7 @@ class Modal extends Component { render() { const { open } = this.state - const { closeOnDimmerClick, closeOnDocumentClick, dimmer } = this.props + const { closeOnDimmerClick, closeOnDocumentClick, dimmer, eventPool } = this.props const mountNode = this.getMountNode() // Short circuit when server side rendering @@ -359,6 +363,7 @@ class Modal extends Component { onMount={this.handlePortalMount} onOpen={this.handleOpen} onUnmount={this.handlePortalUnmount} + eventPool={eventPool} > {this.renderContent(rest)} From dd1dfab4863c426f024200cf9416aa4c590339e2 Mon Sep 17 00:00:00 2001 From: Fox Date: Fri, 22 Sep 2017 11:40:05 -0700 Subject: [PATCH 3/4] Fixing lint errors --- src/addons/Portal/Portal.js | 4 ++-- src/modules/Modal/Modal.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/addons/Portal/Portal.js b/src/addons/Portal/Portal.js index ae0da43f2c..9f60f91bcf 100644 --- a/src/addons/Portal/Portal.js +++ b/src/addons/Portal/Portal.js @@ -129,7 +129,7 @@ class Portal extends Component { closeOnDocumentClick: true, closeOnEscape: true, openOnTriggerClick: true, - eventPool: 'default' + eventPool: 'default', } static autoControlledProps = [ @@ -378,7 +378,7 @@ class Portal extends Component { const { mountNode = isBrowser ? document.body : null, prepend, - eventPool + eventPool, } = this.props this.rootNode = document.createElement('div') diff --git a/src/modules/Modal/Modal.js b/src/modules/Modal/Modal.js index bc62f2b229..87fabbd172 100644 --- a/src/modules/Modal/Modal.js +++ b/src/modules/Modal/Modal.js @@ -138,7 +138,7 @@ class Modal extends Component { dimmer: true, closeOnDimmerClick: true, closeOnDocumentClick: false, - eventPool: 'Modal' + eventPool: 'Modal', } static autoControlledProps = [ From f217ef931d6bb6b9db984861d6846a5ca5458dac Mon Sep 17 00:00:00 2001 From: Fox Date: Fri, 22 Sep 2017 12:59:52 -0700 Subject: [PATCH 4/4] Addressing PR style comments --- src/addons/Portal/Portal.d.ts | 6 +++--- src/addons/Portal/Portal.js | 11 +++++------ src/modules/Modal/Modal.d.ts | 6 +++--- src/modules/Modal/Modal.js | 8 ++++---- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/addons/Portal/Portal.d.ts b/src/addons/Portal/Portal.d.ts index 87c840c129..94b9030146 100644 --- a/src/addons/Portal/Portal.d.ts +++ b/src/addons/Portal/Portal.d.ts @@ -42,6 +42,9 @@ export interface PortalProps { /** Initial value of open. */ defaultOpen?: boolean; + /** Event pool namespace that is used to handle component events. */ + eventPool?: string; + /** The node where the portal should mount. */ mountNode?: any; @@ -95,9 +98,6 @@ export interface PortalProps { /** Controls whether or not the portal should open when mousing over the trigger. */ openOnTriggerMouseEnter?: boolean; - /** Event pool namespace that is used to handle component events. */ - eventPool?: string; - /** Controls whether the portal should be prepended to the mountNode instead of appended. */ prepend?: boolean; diff --git a/src/addons/Portal/Portal.js b/src/addons/Portal/Portal.js index 9f60f91bcf..8d62f08f03 100644 --- a/src/addons/Portal/Portal.js +++ b/src/addons/Portal/Portal.js @@ -62,6 +62,9 @@ class Portal extends Component { /** Initial value of open. */ defaultOpen: PropTypes.bool, + /** Event pool namespace that is used to handle component events */ + eventPool: PropTypes.string, + /** The node where the portal should mount. */ mountNode: PropTypes.any, @@ -115,9 +118,6 @@ class Portal extends Component { /** Controls whether or not the portal should open when mousing over the trigger. */ openOnTriggerMouseEnter: PropTypes.bool, - /** Event pool namespace that is used to handle component events */ - eventPool: PropTypes.string, - /** Controls whether the portal should be prepended to the mountNode instead of appended. */ prepend: PropTypes.bool, @@ -128,8 +128,8 @@ class Portal extends Component { static defaultProps = { closeOnDocumentClick: true, closeOnEscape: true, - openOnTriggerClick: true, eventPool: 'default', + openOnTriggerClick: true, } static autoControlledProps = [ @@ -376,9 +376,9 @@ class Portal extends Component { debug('mountPortal()') const { + eventPool, mountNode = isBrowser ? document.body : null, prepend, - eventPool, } = this.props this.rootNode = document.createElement('div') @@ -398,7 +398,6 @@ class Portal extends Component { if (!isBrowser || !this.rootNode) return debug('unmountPortal()') - const { eventPool } = this.props ReactDOM.unmountComponentAtNode(this.rootNode) diff --git a/src/modules/Modal/Modal.d.ts b/src/modules/Modal/Modal.d.ts index e6775fba52..7109f4823d 100644 --- a/src/modules/Modal/Modal.d.ts +++ b/src/modules/Modal/Modal.d.ts @@ -34,9 +34,6 @@ export interface ModalProps extends PortalProps { /** Whether or not the Modal should close when the document is clicked. */ closeOnDocumentClick?: boolean; - /** Event pool namespace that is used to handle component events */ - eventPool?: string; - /** A Modal can be passed content via shorthand. */ content?: SemanticShorthandItem; @@ -46,6 +43,9 @@ export interface ModalProps extends PortalProps { /** A modal can appear in a dimmer. */ dimmer?: boolean | 'blurring' | 'inverted'; + /** Event pool namespace that is used to handle component events */ + eventPool?: string; + /** A Modal can be passed header via shorthand. */ header?: SemanticShorthandItem; diff --git a/src/modules/Modal/Modal.js b/src/modules/Modal/Modal.js index 87fabbd172..7dcb2bd0ef 100644 --- a/src/modules/Modal/Modal.js +++ b/src/modules/Modal/Modal.js @@ -58,9 +58,6 @@ class Modal extends Component { /** Whether or not the Modal should close when the document is clicked. */ closeOnDocumentClick: PropTypes.bool, - /** Event pool namespace that is used to handle component events */ - eventPool: PropTypes.string, - /** Simple text content for the Modal. */ content: customPropTypes.itemShorthand, @@ -73,6 +70,9 @@ class Modal extends Component { PropTypes.oneOf(['inverted', 'blurring']), ]), + /** Event pool namespace that is used to handle component events */ + eventPool: PropTypes.string, + /** Modal displayed above the content in bold. */ header: customPropTypes.itemShorthand, @@ -357,13 +357,13 @@ class Modal extends Component { closeOnRootNodeClick={closeOnDimmerClick} {...portalProps} className={dimmerClasses} + eventPool={eventPool} mountNode={mountNode} open={open} onClose={this.handleClose} onMount={this.handlePortalMount} onOpen={this.handleOpen} onUnmount={this.handlePortalUnmount} - eventPool={eventPool} > {this.renderContent(rest)}