You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by garrensmith <gi...@git.apache.org> on 2015/02/18 12:32:38 UTC
[GitHub] couchdb-fauxton pull request: Reactify AllDocNumbers
GitHub user garrensmith opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/275
Reactify AllDocNumbers
Convert AllDocNumbers to the react flow
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/garrensmith/couchdb-fauxton react-docs-list
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/275.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 #275
----
commit 654b508e92cff234f6bb5ddcb8eb86a7de1b9ef2
Author: Garren Smith <ga...@gmail.com>
Date: 2015-02-18T11:32:08Z
Reactify AllDocNumbers
Convert AllDocNumbers to the react flow
----
---
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: Reactify AllDocNumbers
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/275#discussion_r24903491
--- Diff: app/addons/documents/tests/docnumbers.componentSpec.react.jsx ---
@@ -0,0 +1,62 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+define([
+ 'api',
+ 'addons/documents/docs-list/components.react',
+ 'testUtils',
+ "react"
+], function (FauxtonAPI, Views, utils, React) {
+ FauxtonAPI.router = new FauxtonAPI.Router([]);
+
+ var assert = utils.assert;
+ var TestUtils = React.addons.TestUtils;
+
+ describe('All Docs Number', function () {
+
+ describe('PerPageSelector', function () {
+ var container, selectorEl, perPageChange;
+
+ beforeEach(function () {
+ perPageChange = sinon.spy();
+ container = document.createElement('div');
+ selectorEl = TestUtils.renderIntoDocument(<Views.PerPageSelector perPageChange={perPageChange} perPage={10} />, container);
+ });
+
+ afterEach(function () {
+ React.unmountComponentAtNode(container);
+ });
+
+ it('on new select calls callback with new page size', function () {
+ var selectEl = $(selectorEl.getDOMNode()).find('#select-per-page')[0];
+ var perPage = 5;
+ TestUtils.Simulate.change(selectEl, {
+ target: {
+ value: perPage
+ }
+ });
+
+ assert.ok(perPageChange.calledWith(perPage));
+ });
+
+ });
+
+
+ });
+
+
+
+
+
+
+
+
--- End diff --
whitespace attack!
---
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: Reactify AllDocNumbers
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/275#discussion_r24903260
--- Diff: app/addons/documents/routes-documents.js ---
@@ -433,8 +415,9 @@ function(app, FauxtonAPI, BaseRoute, Documents, Changes, Index, DocEditor, Datab
// and update the page size
this.perPage = perPage;
this.documentsView.forceRender();
+ this.documentsView.perPageDefault = perPage;
this.documentsView.collection.pageSizeReset(perPage, {fetch: false});
- this.allDocsNumber.forceRender();
+ //this.allDocsNumber.forceRender();
--- End diff --
i guess we can delete that
---
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: Reactify AllDocNumbers
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/275#issuecomment-74890632
This is really clean. Great stuff, Garren.
---
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: Reactify AllDocNumbers
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/275#discussion_r24903762
--- Diff: app/addons/documents/docs-list/components.react.jsx ---
@@ -0,0 +1,119 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+define([
+ "api",
+ "react",
+ 'addons/documents/docs-list/stores',
+ 'addons/documents/docs-list/actions'
+ ], function (FauxtonAPI, React, Stores, Actions) {
+ var allDocsListStore = Stores.allDocsListStore;
+
+ var PerPageSelector = React.createClass({
+
+ perPageChange: function (e) {
+ var perPage = parseInt(e.target.value, 10);
+ this.props.perPageChange(perPage);
+ },
+
+ render: function () {
+ return (
+ <div id="per-page">
--- End diff --
if we want to build more small components in the future, we should try to avoid ids as selectors as the elements.
i think for this element it has no impact, just a thing i realized and wanted to mention
---
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: Reactify AllDocNumbers
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/275#discussion_r24903614
--- Diff: app/addons/documents/docs-list/components.react.jsx ---
@@ -0,0 +1,119 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+define([
+ "api",
+ "react",
+ 'addons/documents/docs-list/stores',
+ 'addons/documents/docs-list/actions'
+ ], function (FauxtonAPI, React, Stores, Actions) {
+ var allDocsListStore = Stores.allDocsListStore;
+
+ var PerPageSelector = React.createClass({
+
+ perPageChange: function (e) {
+ var perPage = parseInt(e.target.value, 10);
+ this.props.perPageChange(perPage);
+ },
+
+ render: function () {
+ return (
+ <div id="per-page">
+ <label htmlFor="select-per-page" className="drop-down inline">
+ Per page:
+ <select id="select-per-page" onChange={this.perPageChange} value={this.props.perPage.toString()} className="input-small">
+ <option value="5">5</option>
+ <option value="10">10</option>
+ <option value="20">20</option>
+ <option value="30">30</option>
+ <option value="50">50</option>
+ <option value="100">100</option>
+ </select>
+ </label>
+ </div>
+ );
+ }
+
+ });
+
+ var AllDocsNumber = React.createClass({
+
+ getStoreState: function () {
+ return {
+ totalRows: allDocsListStore.getTotalRows(),
+ pageStart: allDocsListStore.getPageStart(),
+ pageEnd: allDocsListStore.getPageEnd(),
+ updateSeq: allDocsListStore.getUpdateSeq(),
+ perPage: allDocsListStore.getPerPage()
+ };
+ },
+
+ getInitialState: function () {
+ return this.getStoreState();
+ },
+
+ componentDidMount: function() {
+ allDocsListStore.on('change', this.onChange, this);
+ },
+
+ componentWillUnmount: function() {
+ allDocsListStore.off('change', this.onChange);
+ },
+
+ onChange: function () {
+ this.setState(this.getStoreState());
+ },
+
+ pageNumber: function () {
+ if (this.state.totalRows === 0) {
+ return <p> Showing 0 documents. </p>;
+ } else {
+ return <p>Showing {this.state.pageStart} - {this.state.pageEnd}</p>;
+ }
--- End diff --
the else got a bit unformatted and as you are returning early, you can even omit the else
---
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: Reactify AllDocNumbers
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/275#issuecomment-75551421
Merged
---
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: Reactify AllDocNumbers
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/275#issuecomment-74903890
@benkeen thanks for the feedback I'm made the fixes.
---
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: Reactify AllDocNumbers
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/275#discussion_r24911863
--- Diff: app/addons/documents/docs-list/components.react.jsx ---
@@ -0,0 +1,119 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+define([
+ "api",
+ "react",
+ 'addons/documents/docs-list/stores',
+ 'addons/documents/docs-list/actions'
+ ], function (FauxtonAPI, React, Stores, Actions) {
+ var allDocsListStore = Stores.allDocsListStore;
+
+ var PerPageSelector = React.createClass({
+
+ perPageChange: function (e) {
+ var perPage = parseInt(e.target.value, 10);
+ this.props.perPageChange(perPage);
+ },
+
+ render: function () {
+ return (
+ <div id="per-page">
+ <label htmlFor="select-per-page" className="drop-down inline">
+ Per page:
+ <select id="select-per-page" onChange={this.perPageChange} value={this.props.perPage.toString()} className="input-small">
+ <option value="5">5</option>
+ <option value="10">10</option>
+ <option value="20">20</option>
+ <option value="30">30</option>
+ <option value="50">50</option>
+ <option value="100">100</option>
+ </select>
+ </label>
+ </div>
+ );
+ }
+
+ });
+
+ var AllDocsNumber = React.createClass({
+
+ getStoreState: function () {
+ return {
+ totalRows: allDocsListStore.getTotalRows(),
+ pageStart: allDocsListStore.getPageStart(),
+ pageEnd: allDocsListStore.getPageEnd(),
+ updateSeq: allDocsListStore.getUpdateSeq(),
+ perPage: allDocsListStore.getPerPage()
+ };
+ },
+
+ getInitialState: function () {
+ return this.getStoreState();
+ },
+
+ componentDidMount: function() {
+ allDocsListStore.on('change', this.onChange, this);
+ },
+
+ componentWillUnmount: function() {
+ allDocsListStore.off('change', this.onChange);
+ },
+
+ onChange: function () {
+ this.setState(this.getStoreState());
+ },
+
+ pageNumber: function () {
+ if (this.state.totalRows === 0) {
+ return <p> Showing 0 documents. </p>;
+ }
+
+ return <p>Showing {this.state.pageStart} - {this.state.pageEnd}</p>;
+ },
+
+ updateSequence: function () {
+ if (this.state.updateSeq) {
+ return <span> Update Sequence: {this.state.updateSeq} </span>;
+ }
+ },
+
+ perPageChange: function (perPage) {
+ Actions.updatePerPage(perPage);
+ },
+
+ render: function () {
+ return (
+ <div>
+ <div className="index-indicator">
+ {this.pageNumber()}
+ {this.updateSequence()}
--- End diff --
This is a nice idea. I've been creating separate components (e.g. one for pageNum & one for sequence), which meant more boilerplate code. This is better for small things.
---
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: Reactify AllDocNumbers
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/275#discussion_r24912327
--- Diff: app/addons/documents/docs-list/stores.js ---
@@ -0,0 +1,100 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+define([
+ 'api',
+ 'addons/documents/docs-list/actiontypes'
+],function (FauxtonAPI, ActionTypes) {
+
+ var Stores = {};
+
+ Stores.AllDocsListStore = FauxtonAPI.Store.extend({
+ initialize: function () {
+ this._totalRows = 0;
+ this._updateSeq = false;
+ this._pageStart = 0;
+ this._pageEnd = 20;
+ this._newView = false;
+ this._perPage = FauxtonAPI.constants.MISC.DEFAULT_PAGE_SIZE;
+ },
+
+ collectionChanged: function (collection, pagination, perPage) {
+ if (!this._newView) {
+ this._totalRows = collection.length;
+ this._updateSeq = collection.updateSeq();
+ }
+
+ this._pageStart = pagination.pageStart();
+ this._pageEnd = pagination.pageEnd();
+ this._perPage = perPage;
+ this._pagination = pagination;
+ },
+
+ getPagination: function () {
+ return this._pagination;
+ },
+
+ getPerPage: function () {
+ return this._perPage;
+ },
+
+ setPerPage: function (perPage) {
+ this._perPage = perPage;
+ },
+
+ getTotalRows: function () {
+ return this._totalRows;
+ },
+
+ getPageStart: function () {
+ return this._pageStart;
+ },
+
+ getPageEnd: function () {
+ return this._pageEnd;
+ },
+
+ getUpdateSeq: function () {
+ return this._updateSeq;
+ },
+
+ dispatch: function (action) {
+
+ switch (action.type) {
+ case ActionTypes.SHOW_ALL_DOCS_NUMBER:
+ this.showAllDocsNumber(action.options);
+ this.triggerChange();
+ break;
+
--- End diff --
Possibly legacy? I don't see `SHOW_ALL_DOCS_NUMBER` triggered anywhere & showAllDocsNumber doesn't exist.
---
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: Reactify AllDocNumbers
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/275#discussion_r24903284
--- Diff: app/addons/documents/routes-documents.js ---
@@ -328,17 +319,8 @@ function(app, FauxtonAPI, BaseRoute, Documents, Changes, Index, DocEditor, Datab
});
this.setView('#documents-pagination', this.pagination);
- this.allDocsNumber = new Documents.Views.AllDocsNumber({
- collection: options.indexedDocs,
- pagination: this.pagination,
- perPageDefault: this.perPageDefault
- });
-
- this.setView('#item-numbers', this.allDocsNumber);
-
--- End diff --
:heart_eyes:
---
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: Reactify AllDocNumbers
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/275#discussion_r24903468
--- Diff: app/addons/documents/tests/docnumbers.componentSpec.react.jsx ---
@@ -0,0 +1,62 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+define([
+ 'api',
+ 'addons/documents/docs-list/components.react',
+ 'testUtils',
+ "react"
+], function (FauxtonAPI, Views, utils, React) {
+ FauxtonAPI.router = new FauxtonAPI.Router([]);
+
+ var assert = utils.assert;
+ var TestUtils = React.addons.TestUtils;
+
+ describe('All Docs Number', function () {
+
+ describe('PerPageSelector', function () {
+ var container, selectorEl, perPageChange;
+
+ beforeEach(function () {
+ perPageChange = sinon.spy();
+ container = document.createElement('div');
+ selectorEl = TestUtils.renderIntoDocument(<Views.PerPageSelector perPageChange={perPageChange} perPage={10} />, container);
--- End diff --
just a suggestion as this line gets cut of:
```js
selectorEl = TestUtils.renderIntoDocument(
<Views.PerPageSelector
perPageChange={perPageChange}
perPage={10} />,
container
);
```
---
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: Reactify AllDocNumbers
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/275#issuecomment-74872897
Wow, great work!
I just have some minor style suggestions, I am quite impressed :)
I also realized that we should try to avoid using ids as selector in the future to increase reusability, in my opinion no need to fix that here as I hardly guess that we will keep having just 1 perpage selector in the future on a page, but we should keep that in mind for future components.
---
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: Reactify AllDocNumbers
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/275#discussion_r24903267
--- Diff: app/addons/documents/routes-documents.js ---
@@ -443,7 +426,7 @@ function(app, FauxtonAPI, BaseRoute, Documents, Changes, Index, DocEditor, Datab
this.documentsView.collection.reset(collection);
this.documentsView.forceRender();
- this.allDocsNumber.forceRender();
+ //this.allDocsNumber.forceRender();
--- End diff --
i guess we can delete that
---
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: Reactify AllDocNumbers
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/275#issuecomment-75534212
+1, rebase & merge after travis is green :)
---
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: Reactify AllDocNumbers
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/275
---
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.
---