You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by benkeen <gi...@git.apache.org> on 2016/01/19 21:48:25 UTC
[GitHub] couchdb-fauxton pull request: React 14 update
GitHub user benkeen opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/617
React 14 update
- update React to the most recent version (`0.14.6`)
- compatibility update for React Bootstrap (latest version: `0.28.2`)
- various code updates to use ReactDOM instead of React.
N.B. I ran into an issue with testing React Bootstrap modal components. Testing them was problematic in the first place because they work by creating the modals at the top level of the DOM *outside* the wrapper component. As such, you need to locate the elements in a different fashion (see test code). This seems to have been broken with the latest updates to the lib / React itself. I spent a long time investigating a workaround but couldn't find a solution. I'll create this PR in the meantime & return to it when I can.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/benkeen/couchdb-fauxton react14
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/617.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 #617
----
commit 190972c1a29deac946b423e265e8b5acaefae472
Author: Ben Keen <be...@gmail.com>
Date: 2016-01-19T17:49:30Z
Updating React to 0.14.6
commit eab736a221fafe9a295d015f89ceb3c7a761a87e
Author: Ben Keen <be...@gmail.com>
Date: 2016-01-19T18:33:45Z
removing react 14 warnings
commit 396f6b89011e5bf0c5feb40b6904495873568c5f
Author: Ben Keen <be...@gmail.com>
Date: 2016-01-19T18:48:53Z
react-bootstrap updated to 0.28.2
----
---
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: React 14 update
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/617#discussion_r51377269
--- Diff: app/addons/documents/doc-editor/tests/doc-editor.componentsSpec.react.jsx ---
@@ -146,13 +147,13 @@ define([
}
});
- var attachmentNode = $(el.getDOMNode()).find('.view-attachments-section .dropdown-menu li')[0];
+ var attachmentNode = $(ReactDOM.findDOMNode(el)).find('.view-attachments-section .dropdown-menu li')[0];
var attachmentURLactual = $(attachmentNode).find('a').attr('href');
assert.equal(attachmentURLactual, "../../id/_design/test-doc/one.png");
});
-
+/*
--- End diff --
this should get uncommented or deleted
---
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: React 14 update
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/617#discussion_r51295144
--- Diff: app/addons/components/tests/stringEditModalSpec.react.jsx ---
@@ -70,7 +72,7 @@ define([
container
);
var modal = TestUtils.findRenderedComponentWithType(el, Modal);
- var modalEl = React.findDOMNode(modal.refs.modal);
+ var modalEl = ReactDOM.findDOMNode(modal.refs.modal);
TestUtils.Simulate.click($(modalEl).find('#string-edit-save-btn')[0]);
--- End diff --
Thanks Robert, yeah I'm going to use this technique instead. It's just a test: doesn't have to be beautiful, as long as it's tested!
---
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: React 14 update
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/617#issuecomment-173972854
Cool, ok.
---
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: React 14 update
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/617
---
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: React 14 update
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/617#issuecomment-172997059
I opened up a ticket w.r.t. to the issue I'm having accessing the react bootstrap Modals. Hopefully it's a straightforward fix.
https://github.com/react-bootstrap/react-bootstrap/issues/1617
---
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: React 14 update
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/617#discussion_r50973782
--- Diff: app/addons/components/tests/stringEditModalSpec.react.jsx ---
@@ -86,11 +88,13 @@ define([
);
var modal = TestUtils.findRenderedComponentWithType(el, Modal);
- var modalEl = React.findDOMNode(modal.refs.modal);
+ var modalEl = ReactDOM.findDOMNode(modal.refs.modal);
TestUtils.Simulate.click($(modalEl).find('#string-edit-save-btn')[0]);
assert.ok(spy.calledOnce);
--- End diff --
this test makes no sense and has to get fixed
test title is: "replaces "\\n" with actual newlines"
the actual test asserts that a spy was called once, nothing more.
---
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: React 14 update
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/617#issuecomment-173861431
hey ben,
i would delete the tests "event methods called" -- they don't test our logic, they try to test the react bootstrap component.
leaves us just with the "onSave" tests
---
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: React 14 update
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/617#issuecomment-175513585
hi been i took anothe rlook it looks like the component is not rendered and that means it does not get a ref.
things like
```
console.log("->", $(el).html());
console.log($(ReactDOM.findDOMNode(el)).html());
```
return undefined or empty dom nodes
hope that helps!
---
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: React 14 update
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/617#issuecomment-178088136
+1 great work!
---
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: React 14 update
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/617#issuecomment-177021355
Alright! This should finally be green, @robertkowalski. :)
I moved the newline test code into a separate helper. Ace wouldn't run in mocha environment, so this allows it to be tested in isolation. Beyond that it now uses simple $('body') tests to locate the react modals, and I patched up a few other minor things here and there.
---
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: React 14 update
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/617#issuecomment-177766952
Good catch, @robertkowalski . I uncommented and fixed those modal tests in a rather kludgy manner, but hey! Should be green again. :)
---
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: React 14 update
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/617#discussion_r50973867
--- Diff: app/addons/components/tests/stringEditModalSpec.react.jsx ---
@@ -70,7 +72,7 @@ define([
container
);
var modal = TestUtils.findRenderedComponentWithType(el, Modal);
- var modalEl = React.findDOMNode(modal.refs.modal);
+ var modalEl = ReactDOM.findDOMNode(modal.refs.modal);
TestUtils.Simulate.click($(modalEl).find('#string-edit-save-btn')[0]);
--- End diff --
the only way how i found th button is:
```js
console.log($('body').find('#string-edit-save-btn'));
```
---
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.
---