You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by benkeen <gi...@git.apache.org> on 2015/04/15 07:01:06 UTC
[GitHub] couchdb-fauxton pull request: Doc Workflow improvements
GitHub user benkeen opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/382
Doc Workflow improvements
*** This is just an IDEA. This is not a finished PR. I think the
improvements it makes to the UX is very worthwhile but the
implementation is questionable in places. Intended for discussion
only. ***
_______________________
This PR is part of Doc Workflow to improve the overall
UX of navigating around the UI with respect to the
documents.
Changes:
1. Clicking back/cancel from a full page doc editor no longer
limits the results to 20 on the doc list page.
2. Clicking back to a list of documents from the full page doc
editor now returns you to the last PAGE you were on, and the
SCROLL offset of wherever you were on the page as well.
3. Introduces fauxton/memory.js as a way to store information
across multiple page loads.
Notes:
- Perhaps the functionality in memory.js should be moved to
utils? so app.utils.memory.set(), etc. It could also replace
localStorageGet and localStorageSet: memory.set() and get() would
just take an extra param to specify whether it's getting/setting
local storage or transient JS data.
- it’s missing better tests
- there’s an existing bug where when you click from one page to
the next it doesn’t update the scrollTop to 0 on the scrollable.
That’s unrelated to this ticket.
Big flaws:
- The cloudant.pagingcollection.js file change. Why is that a
separate external lib? I ran into serious problem trying to persuade
it to re-initialize on a non-first page and couldn’t get around it,
hence the modification to the file. There is a solution around it
by circumventing it in the store, but that felt like an even worse
solution.
- I really don’t like having to reference the .scrollable region
(to get + set the scrollTop) within <ResultsScreen />, but I don’t
see any other place to do it now we're dropping Backbone views (i.e.
I need a before/afterRender on the routeobject to know when that
element is available in the page)
- generally I find the whole thing a bit hacky, but I love the UX
improvement.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/benkeen/couchdb-fauxton doc-workflow
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/382.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 #382
----
commit ba491edc91d4ac345cf6e0c52cb13b67896db471
Author: Ben Keen <be...@gmail.com>
Date: 2015-04-15T04:56:48Z
Doc Workflow improvements
*** This is just an IDEA. This is not a finished PR. I think the
improvements it makes to the UX is very worthwhile but the
implementation is questionable in places. Intended for discussion
only. ***
_______________________
This PR is part of Doc Workflow to improve the overall
UX of navigating around the UI with respect to the
documents.
Changes:
1. Clicking back/cancel from a full page doc editor no longer
limits the results to 20 on the doc list page.
2. Clicking back to a list of documents from the full page doc
editor now returns you to the last PAGE you were on, and the
SCROLL offset of wherever you were on the page as well.
3. Introduces fauxton/memory.js as a way to store information
across multiple page loads.
Notes:
- Perhaps the functionality in memory.js should be moved to
utils? so app.utils.memory.set(), etc. It could also replace
localStorageGet and localStorageSet: memory.set() and get() would
just take an extra param to specify whether it's getting/setting
local storage or transient JS data.
- it’s missing better tests
- there’s an existing bug where when you click from one page to
the next it doesn’t update the scrollTop to 0 on the scrollable.
That’s unrelated to this ticket.
Big flaws:
- The cloudant.pagingcollection.js file change. Why is that a
separate external lib? I ran into serious problem trying to persuade
it to re-initialize on a non-first page and couldn’t get around it,
hence the modification to the file. There is a solution around it
by circumventing it in the store, but that felt like an even worse
solution.
- I really don’t like having to reference the .scrollable region
(to get + set the scrollTop) within <ResultsScreen />, but I don’t
see any other place to do it now we're dropping Backbone views (i.e.
I need a before/afterRender on the routeobject to know when that
element is available in the page)
- generally I find the whole thing a bit hacky, but I love the UX
improvement.
----
---
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: [WIP] Doc Workflow improvements
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/382#issuecomment-95328525
Hi @robertkowalski - you mentioned you're pretty familiar with that pagination lib. I explored it some more, but don't see a loophole to get it to properly set `this.paging.hasPrevious` to be true when you're starting on a page other than 1. Any suggestions?
I also investigated updating the lib itself, but none of the unit tests ran for me [possibly outdated dependencies?].
---
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 #382: Doc Workflow improvements
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/382
---
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: [WIP] Doc Workflow improvements
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/382#discussion_r28782823
--- Diff: app/addons/fauxton/components.js ---
@@ -163,6 +164,11 @@ function (app, FauxtonAPI, ace, spin, ZeroClipboard) {
initialize: function (options) {
this.crumbs = options.crumbs;
this.toggleDisabled = options.toggleDisabled || false;
+ this.clickBack = options.clickBack || null;
+ },
+
+ clickBack: function (e) {
+ this.clickBack && this.clickBack(e);
--- End diff --
this.clickBack in line 167 and line 170 don't make sense
---
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: Doc Workflow improvements
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/382#issuecomment-95528272
Maybe it is possible to use the new react router with just parts of Fauxton so we can incrementally switch over. We then could add it to the doclists at first.
Let's chat at the summit about it!
---
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: [WIP] Doc Workflow improvements
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/382#discussion_r28421572
--- Diff: app/addons/documents/routes-documents.js ---
@@ -181,11 +194,14 @@ function (app, FauxtonAPI, BaseRoute, Documents, Changes, ChangesActions, DocEdi
},
cleanup: function () {
+
+ // to ensure garbage collection on the React components
+ this.removeComponent('#dashboard-lower-content');
--- End diff --
I added that because none of the React components were unmounting when leaving the page.
---
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: [WIP] Doc Workflow improvements
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/382#discussion_r28420822
--- Diff: app/addons/documents/routes-documents.js ---
@@ -181,11 +194,14 @@ function (app, FauxtonAPI, BaseRoute, Documents, Changes, ChangesActions, DocEdi
},
cleanup: function () {
+
+ // to ensure garbage collection on the React components
+ this.removeComponent('#dashboard-lower-content');
--- End diff --
This shouldn't be needed. the `FauxtonAPI.RouteObject.prototype.cleanup.apply(this);` should do that for you
---
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: Doc Workflow improvements
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/382#issuecomment-98773768
** We spoke about this. This PR is going to be put on hold for the moment. I'll leave it open. **
---
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 issue #382: Doc Workflow improvements
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the issue:
https://github.com/apache/couchdb-fauxton/pull/382
Unfortunately this PR is stale now. I'm going to close it. Feel free to open it if you want to work on it and bring it up to date.
---
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.
---