From d330c2681339588126d2a8a7c78be4945ffddc46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Mon, 11 Sep 2017 11:57:29 +0200 Subject: [PATCH] react: Different way to do React components This is a different approach to React components suggested by @arturi. Instead of the components adding and removing plugins when mounting and unmounting, plugins are configured at the start, and the components act as slots for the plugins to actually mount in. Because all plugins are still being configured up front here, we don't have to change how the provider `target:` options work, they can install themselves into eg. the Dashboard plugin immediately. The core of this approach is the `UppyWrapper` component, which takes an Uppy instance and a plugin ID, and mounts the plugin inside itself. The other components only provide a default plugin ID. The one change required in Uppy itself for this to work is to the `mount()` functions: now, `mount()` configures `this.target` on the plugins. If plugins do not have a `target:` option, they do not mount on install. Some DOM code also had to be moved into the `mount()` function for the DragDrop component instead of staying in `install()`, but I think it makes sense. (Still have to work out how to make the `DashboardModal` component's `onRequestClose` option work.) --- src/plugins/Dashboard/index.js | 6 +++-- src/plugins/DragDrop/index.js | 17 ++++++++++--- src/plugins/Plugin.js | 36 +++++++++++++++------------ src/plugins/ProgressBar.js | 4 ++- src/plugins/StatusBar/index.js | 4 ++- src/uppy-react/Dashboard.js | 42 ++++---------------------------- src/uppy-react/DashboardModal.js | 33 +++++-------------------- src/uppy-react/DragDrop.js | 30 +++++------------------ src/uppy-react/ProgressBar.js | 28 ++++----------------- src/uppy-react/Wrapper.js | 42 ++++++++++++++++++++++++++++++++ 10 files changed, 108 insertions(+), 134 deletions(-) create mode 100644 src/uppy-react/Wrapper.js diff --git a/src/plugins/Dashboard/index.js b/src/plugins/Dashboard/index.js index ae63943ab..591c0dc0a 100644 --- a/src/plugins/Dashboard/index.js +++ b/src/plugins/Dashboard/index.js @@ -419,8 +419,10 @@ module.exports = class DashboardUI extends Plugin { }}) const target = this.opts.target - const plugin = this - this.target = this.mount(target, plugin) + + if (target) { + this.mount(target, this) + } if (!this.opts.disableStatusBar) { this.core.use(StatusBar, { diff --git a/src/plugins/DragDrop/index.js b/src/plugins/DragDrop/index.js index cb8d66ecb..40cea2dc4 100644 --- a/src/plugins/DragDrop/index.js +++ b/src/plugins/DragDrop/index.js @@ -36,7 +36,7 @@ module.exports = class DragDrop extends Plugin { // Default options const defaultOpts = { - target: '.UppyDragDrop', + target: null, getMetaFromForm: true, locale: defaultLocale } @@ -146,7 +146,13 @@ module.exports = class DragDrop extends Plugin { install () { const target = this.opts.target const plugin = this - this.target = this.mount(target, plugin) + if (target) { + this.mount(target, plugin) + } + } + + mount (...args) { + super.mount(...args) const dndContainer = this.target.querySelector('.UppyDragDrop-container') this.removeDragDropListener = dragDrop(dndContainer, (files) => { @@ -155,8 +161,13 @@ module.exports = class DragDrop extends Plugin { }) } - uninstall () { + unmount (...args) { this.removeDragDropListener() + + super.unmount(...args) + } + + uninstall () { this.unmount() } } diff --git a/src/plugins/Plugin.js b/src/plugins/Plugin.js index f6801f126..87044eb82 100644 --- a/src/plugins/Plugin.js +++ b/src/plugins/Plugin.js @@ -13,7 +13,6 @@ const getFormData = require('get-form-data') * @return {array | string} files or success/fail message */ module.exports = class Plugin { - constructor (core, opts) { this.core = core this.opts = opts || {} @@ -72,34 +71,39 @@ module.exports = class Plugin { this.el = plugin.render(this.core.state) targetElement.appendChild(this.el) + this.target = targetElement + return targetElement } - const Target = target - // Find the target plugin instance. - let targetPlugin - this.core.iteratePlugins((plugin) => { - if (plugin instanceof Target) { - targetPlugin = plugin - return false - } - }) + if (typeof target === 'function') { + const Target = target + // Find the target plugin instance. + let targetPlugin + this.core.iteratePlugins((plugin) => { + if (plugin instanceof Target) { + targetPlugin = plugin + return false + } + }) - if (targetPlugin) { - const targetPluginName = targetPlugin.id - this.core.log(`Installing ${callerPluginName} to ${targetPluginName}`) - return targetPlugin.addTarget(plugin) + if (targetPlugin) { + const targetPluginName = targetPlugin.id + this.core.log(`Installing ${callerPluginName} to ${targetPluginName}`) + this.target = targetPlugin + return targetPlugin.addTarget(plugin) + } } this.core.log(`Not installing ${callerPluginName}`) - - return null + throw new Error(`Invalid target option given to ${callerPluginName}`) } unmount () { if (this.el && this.el.parentNode) { this.el.parentNode.removeChild(this.el) } + this.target = null } install () { diff --git a/src/plugins/ProgressBar.js b/src/plugins/ProgressBar.js index 7f1b84c8e..4ff3762a7 100644 --- a/src/plugins/ProgressBar.js +++ b/src/plugins/ProgressBar.js @@ -37,7 +37,9 @@ module.exports = class ProgressBar extends Plugin { install () { const target = this.opts.target const plugin = this - this.target = this.mount(target, plugin) + if (target) { + this.mount(target, plugin) + } } uninstall () { diff --git a/src/plugins/StatusBar/index.js b/src/plugins/StatusBar/index.js index e08bf47d8..5e5c157f9 100644 --- a/src/plugins/StatusBar/index.js +++ b/src/plugins/StatusBar/index.js @@ -132,7 +132,9 @@ module.exports = class StatusBarUI extends Plugin { install () { const target = this.opts.target const plugin = this - this.target = this.mount(target, plugin) + if (target) { + this.mount(target, plugin) + } } uninstall () { diff --git a/src/uppy-react/Dashboard.js b/src/uppy-react/Dashboard.js index ebdeda267..7110956e4 100644 --- a/src/uppy-react/Dashboard.js +++ b/src/uppy-react/Dashboard.js @@ -1,7 +1,7 @@ const React = require('react') const PropTypes = require('prop-types') const UppyCore = require('../core/Core') -const DashboardPlugin = require('../plugins/Dashboard') +const UppyWrapper = require('./Wrapper') const h = React.createElement @@ -10,46 +10,14 @@ const h = React.createElement * renders the Dashboard inline, so you can put it anywhere you want. */ -class Dashboard extends React.Component { - componentDidMount () { - const uppy = this.props.uppy - const options = Object.assign({}, this.props, { - target: this.container, - inline: true - }) - delete options.uppy - uppy.use(DashboardPlugin, options) - - this.plugin = uppy.getPlugin('DashboardUI') - } - - componentWillUnmount () { - const uppy = this.props.uppy - - uppy.removePlugin(this.plugin) - } - - render () { - return h('div', { - ref: (container) => { - this.container = container - } - }) - } -} +const Dashboard = (props) => + h(UppyWrapper, props) Dashboard.propTypes = { - uppy: PropTypes.instanceOf(UppyCore).isRequired, - maxWidth: PropTypes.number, - maxHeight: PropTypes.number, - semiTransparent: PropTypes.bool, - defaultTabIcon: PropTypes.node, - showProgressDetails: PropTypes.bool, - locale: PropTypes.object + uppy: PropTypes.instanceOf(UppyCore).isRequired } - Dashboard.defaultProps = { - locale: {} + plugin: 'DashboardUI' } module.exports = Dashboard diff --git a/src/uppy-react/DashboardModal.js b/src/uppy-react/DashboardModal.js index 25e8e72c0..6cb5ba35e 100644 --- a/src/uppy-react/DashboardModal.js +++ b/src/uppy-react/DashboardModal.js @@ -1,7 +1,7 @@ const React = require('react') const PropTypes = require('prop-types') const UppyCore = require('../core/Core') -const DashboardPlugin = require('../plugins/Dashboard') +const UppyWrapper = require('./Wrapper') const h = React.createElement @@ -13,14 +13,8 @@ const h = React.createElement class DashboardModal extends React.Component { componentDidMount () { const uppy = this.props.uppy - const options = Object.assign({}, this.props, { - target: this.container, - onRequestHideModal: this.props.onRequestClose - }) - delete options.uppy - uppy.use(DashboardPlugin, options) - this.plugin = uppy.getPlugin('DashboardUI') + this.plugin = uppy.getPlugin(this.props.plugin) if (this.props.open) { this.plugin.showModal() } @@ -34,34 +28,19 @@ class DashboardModal extends React.Component { } } - componentWillUnmount () { - const uppy = this.props.uppy - - uppy.removePlugin(this.plugin) - } - render () { - return h('div', { - ref: (container) => { - this.container = container - } - }) + return h(UppyWrapper, this.props) } } DashboardModal.propTypes = { uppy: PropTypes.instanceOf(UppyCore).isRequired, - maxWidth: PropTypes.number, - maxHeight: PropTypes.number, - semiTransparent: PropTypes.bool, - defaultTabIcon: PropTypes.node, - showProgressDetails: PropTypes.bool, - onRequestClose: PropTypes.func, - locale: PropTypes.object + open: PropTypes.bool, + onRequestClose: PropTypes.func } DashboardModal.defaultProps = { - locale: {} + plugin: 'DashboardUI' } module.exports = DashboardModal diff --git a/src/uppy-react/DragDrop.js b/src/uppy-react/DragDrop.js index 7042dd8c3..2fe7f1733 100644 --- a/src/uppy-react/DragDrop.js +++ b/src/uppy-react/DragDrop.js @@ -1,7 +1,7 @@ const React = require('react') const PropTypes = require('prop-types') -const UppyCore = require('../core/Core') -const DragDropPlugin = require('../plugins/DragDrop') +const UppyCore = require('../core') +const UppyWrapper = require('./Wrapper') const h = React.createElement @@ -10,32 +10,14 @@ const h = React.createElement * uploaded. */ -class DragDrop extends React.Component { - componentDidMount () { - const uppy = this.props.uppy - const options = Object.assign({}, this.props, { - target: this.container - }) - delete options.uppy - uppy.use(DragDropPlugin, options) - } - - render () { - return h('div', { - ref: (container) => { - this.container = container - } - }) - } -} +const DragDrop = (props) => + h(UppyWrapper, props) DragDrop.propTypes = { - uppy: PropTypes.instanceOf(UppyCore).isRequired, - locale: PropTypes.object + uppy: PropTypes.instanceOf(UppyCore).isRequired } - DragDrop.defaultProps = { - locale: {} + plugin: 'DragDrop' } module.exports = DragDrop diff --git a/src/uppy-react/ProgressBar.js b/src/uppy-react/ProgressBar.js index f3bfb1a88..800679271 100644 --- a/src/uppy-react/ProgressBar.js +++ b/src/uppy-react/ProgressBar.js @@ -1,7 +1,7 @@ const React = require('react') const PropTypes = require('prop-types') const UppyCore = require('../core/Core') -const ProgressBarPlugin = require('../plugins/ProgressBar') +const UppyWrapper = require('./Wrapper') const h = React.createElement @@ -10,32 +10,14 @@ const h = React.createElement * uploaded. */ -class ProgressBar extends React.Component { - componentDidMount () { - const uppy = this.props.uppy - const options = Object.assign({}, this.props, { - target: this.container - }) - delete options.uppy - uppy.use(ProgressBarPlugin, options) - } - - render () { - return h('div', { - ref: (container) => { - this.container = container - } - }) - } -} +const ProgressBar = (props) => + h(UppyWrapper, props) ProgressBar.propTypes = { - uppy: PropTypes.instanceOf(UppyCore).isRequired, - locale: PropTypes.object + uppy: PropTypes.instanceOf(UppyCore).isRequired } - ProgressBar.defaultProps = { - locale: {} + plugin: 'ProgressBar' } module.exports = ProgressBar diff --git a/src/uppy-react/Wrapper.js b/src/uppy-react/Wrapper.js new file mode 100644 index 000000000..9b046a013 --- /dev/null +++ b/src/uppy-react/Wrapper.js @@ -0,0 +1,42 @@ +const React = require('react') +const PropTypes = require('prop-types') +const UppyCore = require('../core') + +const h = React.createElement + +class UppyWrapper extends React.Component { + constructor (props) { + super(props) + + this.refContainer = this.refContainer.bind(this) + } + + componentDidMount () { + const plugin = this.props.uppy + .getPlugin(this.props.plugin) + + plugin.mount(this.container, plugin) + } + + componentWillUnmount () { + const plugin = this.props.uppy + .getPlugin(this.props.plugin) + + plugin.unmount() + } + + refContainer (container) { + this.container = container + } + + render () { + return h('div', { ref: this.refContainer }) + } +} + +UppyWrapper.propTypes = { + uppy: PropTypes.instanceOf(UppyCore).isRequired, + plugin: PropTypes.string.isRequired +} + +module.exports = UppyWrapper