You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by robertkowalski <gi...@git.apache.org> on 2016/02/09 18:49:13 UTC
[GitHub] couchdb-fauxton pull request: trays: use higher order components t...
GitHub user robertkowalski opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/642
trays: use higher order components to connect them to stores
this is the first step to get rid of `<Tray />` which uses
`FauxtonAPI.Events` and additionally manages its own state by
accessing the DOM which leads to subtle bugs.
we use composition/higher order components as all trays share the
same wrapper, but have different stores connected.
- change test urls to example.com, which is reserved for testing
- use React.findDOMNode
- trays don't touch the DOM directly any more
COUCHDB-2943
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertkowalski/couchdb-fauxton trays-composition
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/642.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #642
----
commit 7200dce0418ba71dbac08263c2691b7a40ce2726
Author: Robert Kowalski <ro...@apache.org>
Date: 2016-02-09T17:39:55Z
trays: use higher order components to connect them to stores
this is the first step to get rid of <Tray /> which uses
`FauxtonAPI.Events` and additionally manages its own state by
accessing the DOM which leads to subtle bugs.
we use composition/higher order components as all trays share the
same wrapper, but have different stores connected.
- change test urls to example.com, which is reserved for testing
- use React.findDOMNode
- trays don't touch the DOM directly any more
COUCHDB-2943
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: trays: use higher order components t...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/642
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: trays: use higher order components t...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/642#issuecomment-181995502
Other than that one isMounted check, this looks good. +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: trays: use higher order components t...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/642#issuecomment-183379591
@benkeen fixed and rebased
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: trays: use higher order components t...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/642#issuecomment-187690329
merged as 40d956f0787087f47332e11e40f4b65cc9d0ce4a
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: trays: use higher order components t...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/642#issuecomment-183446629
Sweet. +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: trays: use higher order components t...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/642#discussion_r52351786
--- Diff: app/addons/components/react-components.react.jsx ---
@@ -1203,6 +1205,183 @@ function (app, FauxtonAPI, React, ReactDOM, Stores, FauxtonComponents, Helpers,
}
});
+
+ function connectToStores (Component, stores, getStateFromStores) {
+
+ var WrappingElement = React.createClass({
+
+ componentDidMount: function () {
+ stores.forEach(function (store) {
+ store.on('change', this.onChange, this);
+ }.bind(this));
+ },
+
+ componentWillUnmount: function () {
+ stores.forEach(function (store) {
+ store.off('change', this.onChange);
+ }.bind(this));
+ },
+
+ getInitialState: function () {
+ return getStateFromStores(this.props);
+ },
+
+ onChange: function () {
--- End diff --
connectToStores is nice and clean, very cool.
Do we need an `this.isMounted()` check here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: trays: use higher order components t...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/642#discussion_r52731855
--- Diff: app/addons/components/react-components.react.jsx ---
@@ -1203,6 +1205,183 @@ function (app, FauxtonAPI, React, ReactDOM, Stores, FauxtonComponents, Helpers,
}
});
+
+ function connectToStores (Component, stores, getStateFromStores) {
+
+ var WrappingElement = React.createClass({
+
+ componentDidMount: function () {
+ stores.forEach(function (store) {
+ store.on('change', this.onChange, this);
+ }.bind(this));
+ },
+
+ componentWillUnmount: function () {
+ stores.forEach(function (store) {
+ store.off('change', this.onChange);
+ }.bind(this));
+ },
+
+ getInitialState: function () {
+ return getStateFromStores(this.props);
+ },
+
+ onChange: function () {
--- End diff --
good call!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: trays: use higher order components t...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/642#discussion_r52350444
--- Diff: app/addons/components/actions.js ---
@@ -16,30 +16,33 @@ define([
],
function (FauxtonAPI, ActionTypes) {
- function showAPIBar () {
- FauxtonAPI.dispatch({ type: ActionTypes.SHOW_API_BAR });
+ function showAPIBarButton () {
--- End diff --
Nice.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---