Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds the ability to sort table data #416

Merged
merged 1 commit into from May 27, 2015

Conversation

Projects
None yet
3 participants
@anmoljagetia
Copy link
Contributor

commented May 26, 2015

@GordonSmith @mlzummo Please review!
Adds the ability to sort the data in the table based on column selected.
Demo at : http://rawgit.com/anmoljagetia/Visualization/tableSort/demos/dermatology.html?src/other/Table

cursor : pointer;
}


This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 26, 2015

Member

Remove additional blank lines

}


.thIcon {

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 26, 2015

Member

Should be namespaced with .other_Table

this._columns = [];
this._paginator = new Paginator();
this._faChar = new FAChar();

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 26, 2015

Member

faChar not used.

context._order = -1;
}
context.click(column)
;

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 26, 2015

Member

move semicolon up a line

th.select(".thIcon")
.text(function (column) {
if (context._order === -1) {
// return context._currentSort === column ? "\uf106" : "";

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 26, 2015

Member

remove commented code.

@anmoljagetia anmoljagetia force-pushed the anmoljagetia:tableSort branch from 00eceee to 1a7d704 May 26, 2015

@@ -138,8 +175,31 @@
};


Table.prototype.click = function (d) {
};
Table.prototype.click = function (column) {

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 26, 2015

Member

This "click" method is already in use by the data row. Can you rename it to _headerClick or such like (the _ is to indicate its private).

@anmoljagetia anmoljagetia force-pushed the anmoljagetia:tableSort branch 2 times, most recently from c61b93b to 55ae441 May 26, 2015

@@ -6,13 +6,18 @@
}

.other_Table th {
padding:5px 10px;
padding:5px 10px;
background-color:#1f77b4;
border-width: 1px;
border-style: solid;
border-color: #a9c6c9;

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 26, 2015

Member

indentation is off...

This comment has been minimized.

Copy link
@anmoljagetia

anmoljagetia May 26, 2015

Author Contributor

My text editor (sublime text), doesn't seem to show anything wrong, nor does vim :/
selection_005
selection_006

This comment has been minimized.

Copy link
@mzummo

mzummo May 26, 2015

Contributor

there is setting for sublime to remove all whitespace on save (that helps a lot) (I have had the same issue as you and this has fixed it)

Preferences-> User prefs ->
{
"trim_trailing_white_space_on_save": true
}

sometimes i need to open in a new editor though like notepad++ or netbeans to correct it first timearound

This comment has been minimized.

Copy link
@anmoljagetia

anmoljagetia May 26, 2015

Author Contributor

@mlzummo thanks :) I think it fixes the issue! Don't really know what causes it.

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 26, 2015

Member

I think that setting just removes trailing white spaces?

}
}(this, function (HTMLWidget, Paginator) {
}(this, function (d3, HTMLWidget, FAChar, Paginator) {

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 26, 2015

Member

FAChar not used

@GordonSmith

This comment has been minimized.

Copy link
Member

commented May 26, 2015

Just the minor indentation and the unused FAChar, then I can pull.

@anmoljagetia anmoljagetia force-pushed the anmoljagetia:tableSort branch from 55ae441 to 3c10f31 May 26, 2015

@GordonSmith

This comment has been minimized.

Copy link
Member

commented May 26, 2015

There should be a setting to convert tabs to spaces (1 tab === 4 spaces), which will fix the indentation.

I think there is a lint setting for that, I should set!

@anmoljagetia anmoljagetia force-pushed the anmoljagetia:tableSort branch from 3c10f31 to 8e6e602 May 27, 2015

th.select(".thIcon")
.text(function (column) {
if (context._order === -1) {
return context._currentSort === column ? "\t" + "\uf078" : "";

This comment has been minimized.

Copy link
@GordonSmith

GordonSmith May 27, 2015

Member

I don't think HTML support tab spaces "\t" - if you want a "gap" you should either style the span or add a " "

Adds the ability to sort table data
Signed-off-by: Anmol Jagetia <anmoljagetia@gmail.com>

@anmoljagetia anmoljagetia force-pushed the anmoljagetia:tableSort branch from 8e6e602 to c1b406b May 27, 2015

@GordonSmith

This comment has been minimized.

Copy link
Member

commented May 27, 2015

Looks good.

GordonSmith added a commit that referenced this pull request May 27, 2015

Merge pull request #416 from anmoljagetia/tableSort
Adds the ability to sort table data

@GordonSmith GordonSmith merged commit 3498bae into hpcc-systems:master May 27, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.