You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by robertkowalski <gi...@git.apache.org> on 2014/04/08 21:48:24 UTC
[GitHub] couchdb pull request: 2125 sort tasks refactor
GitHub user robertkowalski opened a pull request:
https://github.com/apache/couchdb/pull/203
2125 sort tasks refactor
I refactored the Activetasks.
I was able to delete far more code than adding new code (even with the whitepsace removals and code formatting) and implementing new stuff that operates on the collection should now get pretty straightforward.
I separated the remove whitespace commit and another one, but I can squash / rebase.
Here is what I did:
- do not use a model as a kind of collection, directly operate on the
collection which gets filtered for the view
- remove the view DataSection which acted as layer to fetch collections from
the model and passed it to a view
- remove not used html-template
- format code
This is the base for COUCHDB-2125 but the feature might need some concept and style which could take some time
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertkowalski/couchdb 2125-sort-tasks-refactor
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb/pull/203.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 #203
----
commit 75f9a04e116fcee353e1d81dd61ff69867d868f5
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2014-04-06T16:17:36Z
Fauxton: reformat code & remove whitespace
commit 887f004ffbbb1ff04d2e7f686f17b0068228367c
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2014-04-06T21:15:16Z
Fauxton: refactor ActiveTasks
- do not use a model as a collection, directly operate on the
collection which gets filtered for the view
- remove the view which acted as layer to fetch collections from
the model and passed it to a view
- remove not used html-template
- format code
commit 7090bd51863f9be2bb588a399274a534946dff1a
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2014-04-06T21:42:04Z
Fauxton: do not load unnessecary d3 & backbone
----
---
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 pull request: 2125 sort tasks refactor
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11454797
--- Diff: src/fauxton/app/addons/activetasks/resources.js ---
@@ -12,104 +12,44 @@
define([
"app",
- "backbone",
"addons/fauxton/base",
- "d3"
],
-function (app, backbone, Fauxton) {
+function (app, Fauxton) {
var Active = {},
apiv = app.versionAPI;
app.taskSortBy = 'type';
- Active.Task = Backbone.Model.extend({
- initialize: function() {
- this.set({"id": this.get('pid')});
- }
- });
-
-// ALL THE TASKS
- Active.Tasks = Backbone.Model.extend({
- alltypes: {
- "all": "All tasks",
- "replication": "Replication",
- "database_compaction":" Database Compaction",
- "indexer": "Indexer",
- "view_compaction": "View Compaction"
- },
- documentation: "_active_tasks",
- url: function (context) {
- if (context === "apiurl"){
- return window.location.origin + '/_active_tasks';
- } else {
- return app.host + '/_active_tasks';
- }
- },
- fetch: function (options) {
- var fetchoptions = options || {};
- fetchoptions.cache = false;
- return Backbone.Model.prototype.fetch.call(this, fetchoptions);
- },
- parse: function(resp){
- var types = this.getUniqueTypes(resp),
- that = this;
- var typeCollections = _.reduce(types, function (collection, val, key) {
- collection[key] = new Active.AllTasks(that.sortThis(resp, key));
- return collection;
- }, {});
+ Active.events = {};
+ _.extend(Active.events, Backbone.Events);
- typeCollections.all = new Active.AllTasks(resp);
-
- this.set(typeCollections); //now set them all to the model
- },
- getUniqueTypes: function(resp){
- var types = this.alltypes;
-
- _.each(resp, function(type){
- if( typeof(types[type.type]) === "undefined"){
- types[type.type] = type.type.replace(/_/g,' ');
- }
- },this);
-
- this.alltypes = types;
- return types;
- },
- sortThis: function(resp, type){
- return _.filter(resp, function(item) { return item.type === type; });
- },
- changeView: function (view){
- this.set({
- "currentView": view
- });
- },
- getCurrentViewData: function(){
- var currentView = this.get('currentView');
- return this.get(currentView);
- },
- getDatabaseCompactions: function(){
- return this.get('databaseCompactions');
- },
- getIndexes: function(){
- return this.get('indexes');
- },
- getViewCompactions: function(){
- return this.get('viewCompactions');
+ Active.Task = Backbone.Model.extend({
+ initialize: function () {
+ this.set({"id": this.get('pid')});
--- End diff --
Good to know. It is a leftover from the old code, but I will change 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 pull request: 2125 sort tasks refactor
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11455487
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
+ },
+
+ beforeRender: function () {
+ this.filterAndRender(this.filter);
+ },
+
+ filterAndRender: function (view) {
+ var self = this;
+ this.filter = view;
+ this.removeView("#tasks_go_here");
--- End diff --
changed that to a class, I just reused this element from the older code.
---
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 pull request: 2125 sort tasks refactor
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb/pull/203#issuecomment-42812458
+1 if you haven't merged this yet. You should merge it. Nice refactoring. Thanks.
---
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 pull request: 2125 sort tasks refactor
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11425862
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
+ },
+
+ beforeRender: function () {
+ this.filterAndRender(this.filter);
+ },
+
+ filterAndRender: function (view) {
+ var self = this;
+ this.filter = view;
+ this.removeView("#tasks_go_here");
+
+ this.collection.forEach(function (item) {
+ if (self.filter !== "all" && item.get("type") !== self.filter) {
+ return;
+ }
+ var view = new Views.TableDetail({
+ model: item
+ });
+ this.insertView("#tasks_go_here", view).render();
--- End diff --
Because this is in a beforeRender, you don't have to call render. As it will get automatically rendered by `backbone.layoutmanager`
---
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 pull request: 2125 sort tasks refactor
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11455003
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
+ },
+
+ beforeRender: function () {
+ this.filterAndRender(this.filter);
+ },
+
+ filterAndRender: function (view) {
+ var self = this;
+ this.filter = view;
+ this.removeView("#tasks_go_here");
+
+ this.collection.forEach(function (item) {
+ if (self.filter !== "all" && item.get("type") !== self.filter) {
+ return;
+ }
+ var view = new Views.TableDetail({
+ model: item
+ });
+ this.insertView("#tasks_go_here", view).render();
+ }, this);
+ },
+
+ afterRender: function () {
+ Events.bind("update:poll", this.setPolling, this);
+ this.setPolling();
+ },
+
+ establish: function () {
+ return [this.collection.fetch()];
+ },
+
+ serialize: function () {
+ return {
+ currentView: this.currentView,
+ collection: this.collection
+ };
+ },
+
+ sortByType: function (e) {
+ var currentTarget = e.currentTarget,
+ datatype = $(currentTarget).attr("data-type");
--- End diff --
Good catch!
---
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 pull request: 2125 sort tasks refactor
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb/pull/203#issuecomment-39938213
Hey @robertkowalski,
I haven't had a chance to test it yet. But I've taken a look at the code and given some feedback. Most of the feedback is just getting it to more of the style of the rest of Fauxton. This is a great job, even more so because we have never actually explained this code to you. You are rocking this.
I will try and test this a little later and report back.
---
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 pull request: 2125 sort tasks refactor
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11455457
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
+ },
+
+ beforeRender: function () {
+ this.filterAndRender(this.filter);
+ },
+
+ filterAndRender: function (view) {
+ var self = this;
--- End diff --
changed!
---
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 pull request: 2125 sort tasks refactor
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb/pull/203#issuecomment-43256340
merged! :) thanks for the reviews you've done! i will merge slowly, as I am having some merge conflicts.
---
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 pull request: 2125 sort tasks refactor
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:
https://github.com/apache/couchdb/pull/203
---
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 pull request: 2125 sort tasks refactor
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11425761
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
--- End diff --
With newer versions of Backbone its better to do. `this.listenTo(this.collection, "reset", this.render)`. The event will then be garbage collected when the view is removed. It also automatically binds the `this` context. http://backbonejs.org/#Events-listenTo
---
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 pull request: 2125 sort tasks refactor
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11425783
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
+ },
+
+ beforeRender: function () {
+ this.filterAndRender(this.filter);
+ },
+
+ filterAndRender: function (view) {
+ var self = this;
--- End diff --
We have tried to stick to a convention of using `var that = this`. I used to use `self` too, but we got to choose a convention and `that` is more javascripty.
---
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 pull request: 2125 sort tasks refactor
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11653349
--- Diff: src/fauxton/app/addons/activetasks/routes.js ---
@@ -19,36 +19,37 @@ define([
function (app, FauxtonAPI, Activetasks, Views) {
- var ActiveTasksRouteObject = FauxtonAPI.RouteObject.extend({
+ var ActiveTasksRouteObject = FauxtonAPI.RouteObject.extend({
layout: "with_sidebar",
+
routes: {
"activetasks/:id": "defaultView",
"activetasks": "defaultView"
},
+
selectedHeader: 'Active Tasks',
+
crumbs: [
- {"name": "Active tasks", "link": "activetasks"}
+ {"name": "Active tasks", "link": "activetasks"}
],
- apiUrl: function(){
- return [this.newtasks.url("apiurl"), this.newtasks.documentation];
- },
+
+ apiUrl: function () {
+ return [this.allTasks.url("apiurl"), this.allTasks.documentation];
+ },
roles: ["_admin"],
- defaultView: function(id){
- this.newtasks = new Activetasks.Tasks({
- currentView: "all",
- id:'activeTasks'
- });
- this.setView("#sidebar-content", new Views.TabMenu({
- currentView: "all",
- model: this.newtasks
- }));
-
- this.setView("#dashboard-content", new Views.DataSection({
- model: this.newtasks,
+ initialize: function () {
+ this.allTasks = new Activetasks.AllTasks({currentView: "all"});
--- End diff --
`{currentView: "all"}` i do not need 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 pull request: 2125 sort tasks refactor
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11425898
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
+ },
+
+ beforeRender: function () {
+ this.filterAndRender(this.filter);
+ },
+
+ filterAndRender: function (view) {
+ var self = this;
+ this.filter = view;
+ this.removeView("#tasks_go_here");
+
+ this.collection.forEach(function (item) {
+ if (self.filter !== "all" && item.get("type") !== self.filter) {
+ return;
+ }
+ var view = new Views.TableDetail({
+ model: item
+ });
+ this.insertView("#tasks_go_here", view).render();
+ }, this);
+ },
+
+ afterRender: function () {
+ Events.bind("update:poll", this.setPolling, this);
+ this.setPolling();
+ },
+
+ establish: function () {
+ return [this.collection.fetch()];
+ },
+
+ serialize: function () {
+ return {
+ currentView: this.currentView,
+ collection: this.collection
+ };
+ },
+
+ sortByType: function (e) {
+ var currentTarget = e.currentTarget,
+ datatype = $(currentTarget).attr("data-type");
--- End diff --
Its best to add `this`, so it should be `this.$(currentTarget).attr("data-type");`. That way jQuery is bound to the views context.
---
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 pull request: 2125 sort tasks refactor
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11425423
--- Diff: src/fauxton/app/addons/activetasks/resources.js ---
@@ -12,104 +12,44 @@
define([
"app",
- "backbone",
"addons/fauxton/base",
- "d3"
],
-function (app, backbone, Fauxton) {
+function (app, Fauxton) {
var Active = {},
apiv = app.versionAPI;
app.taskSortBy = 'type';
- Active.Task = Backbone.Model.extend({
- initialize: function() {
- this.set({"id": this.get('pid')});
- }
- });
-
-// ALL THE TASKS
- Active.Tasks = Backbone.Model.extend({
- alltypes: {
- "all": "All tasks",
- "replication": "Replication",
- "database_compaction":" Database Compaction",
- "indexer": "Indexer",
- "view_compaction": "View Compaction"
- },
- documentation: "_active_tasks",
- url: function (context) {
- if (context === "apiurl"){
- return window.location.origin + '/_active_tasks';
- } else {
- return app.host + '/_active_tasks';
- }
- },
- fetch: function (options) {
- var fetchoptions = options || {};
- fetchoptions.cache = false;
- return Backbone.Model.prototype.fetch.call(this, fetchoptions);
- },
- parse: function(resp){
- var types = this.getUniqueTypes(resp),
- that = this;
- var typeCollections = _.reduce(types, function (collection, val, key) {
- collection[key] = new Active.AllTasks(that.sortThis(resp, key));
- return collection;
- }, {});
+ Active.events = {};
+ _.extend(Active.events, Backbone.Events);
- typeCollections.all = new Active.AllTasks(resp);
-
- this.set(typeCollections); //now set them all to the model
- },
- getUniqueTypes: function(resp){
- var types = this.alltypes;
-
- _.each(resp, function(type){
- if( typeof(types[type.type]) === "undefined"){
- types[type.type] = type.type.replace(/_/g,' ');
- }
- },this);
-
- this.alltypes = types;
- return types;
- },
- sortThis: function(resp, type){
- return _.filter(resp, function(item) { return item.type === type; });
- },
- changeView: function (view){
- this.set({
- "currentView": view
- });
- },
- getCurrentViewData: function(){
- var currentView = this.get('currentView');
- return this.get(currentView);
- },
- getDatabaseCompactions: function(){
- return this.get('databaseCompactions');
- },
- getIndexes: function(){
- return this.get('indexes');
- },
- getViewCompactions: function(){
- return this.get('viewCompactions');
+ Active.Task = Backbone.Model.extend({
+ initialize: function () {
+ this.set({"id": this.get('pid')});
--- End diff --
@robertkowalski you can set the idAttribute of a model so that you don't need to do this. http://backbonejs.org/#Model-idAttribute
---
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 pull request: 2125 sort tasks refactor
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11425987
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
+ },
+
+ beforeRender: function () {
+ this.filterAndRender(this.filter);
+ },
+
+ filterAndRender: function (view) {
+ var self = this;
+ this.filter = view;
+ this.removeView("#tasks_go_here");
+
+ this.collection.forEach(function (item) {
+ if (self.filter !== "all" && item.get("type") !== self.filter) {
+ return;
+ }
+ var view = new Views.TableDetail({
+ model: item
+ });
+ this.insertView("#tasks_go_here", view).render();
+ }, this);
+ },
+
+ afterRender: function () {
+ Events.bind("update:poll", this.setPolling, this);
+ this.setPolling();
+ },
+
+ establish: function () {
+ return [this.collection.fetch()];
+ },
+
+ serialize: function () {
+ return {
+ currentView: this.currentView,
+ collection: this.collection
+ };
+ },
+
+ sortByType: function (e) {
+ var currentTarget = e.currentTarget,
+ datatype = $(currentTarget).attr("data-type");
+
+ this.collection.sortByColumn(datatype);
+ this.render();
+ },
+
+ setPolling: function () {
+ var self = this;
--- End diff --
I think you could rewrite this whole function to not use `self` aka `that`. We try to use `that` as our last case if there is no other way of doing it. So this function could be:
setPolling: function () {
var collection = this.collection;
clearInterval(pollingInfo.intervalId);
pollingInfo.intervalId = setInterval(function () {
collection.fetch({reset: true});
}, pollingInfo.rate * 1000);
}
---
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 pull request: 2125 sort tasks refactor
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11455771
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
+ },
+
+ beforeRender: function () {
+ this.filterAndRender(this.filter);
+ },
+
+ filterAndRender: function (view) {
+ var self = this;
+ this.filter = view;
+ this.removeView("#tasks_go_here");
+
+ this.collection.forEach(function (item) {
+ if (self.filter !== "all" && item.get("type") !== self.filter) {
+ return;
+ }
+ var view = new Views.TableDetail({
+ model: item
+ });
+ this.insertView("#tasks_go_here", view).render();
--- End diff --
I had to render in this case, because I was calling the method in https://github.com/robertkowalski/couchdb/blob/2125-sort-tasks-refactor/src/fauxton/app/addons/activetasks/views.js#L45 but I changed 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 pull request: 2125 sort tasks refactor
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/203#discussion_r11425831
--- Diff: src/fauxton/app/addons/activetasks/views.js ---
@@ -28,23 +28,105 @@ function (app, FauxtonAPI, activetasks) {
Views.Events = _.extend(Events, Backbone.Events);
+ Views.View = FauxtonAPI.View.extend({
+ tagName: "table",
+
+ className: "table table-bordered table-striped active-tasks",
+
+ template: "addons/activetasks/templates/table",
+
+ events: {
+ "click th": "sortByType"
+ },
+
+ filter: "all",
+
+ initialize: function (options) {
+ ActiveTasks.events.on("tasks:filter", this.filterAndRender, this);
+ this.collection.bind("reset", _.bind(this.render, this));
+ },
+
+ beforeRender: function () {
+ this.filterAndRender(this.filter);
+ },
+
+ filterAndRender: function (view) {
+ var self = this;
+ this.filter = view;
+ this.removeView("#tasks_go_here");
--- End diff --
for css we use the convention of hyphens. So `#tasks_go_here` should be `#tasks-go-here`. Also something we have recently decided on is to prefix andy css id's or classes with `js-` if it is used in the javascript. So that id should be `#js-tasks-go-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 pull request: 2125 sort tasks refactor
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb/pull/203#issuecomment-40010539
Thanks for the great suggestions! Changed that and added it as a separate commit as this refactor is huge enough :)
---
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.
---