Skip to content

Commit

Permalink
fix(sortable-table): sorts on first sortable column
Browse files Browse the repository at this point in the history 
instead of on the first column regardless of whether it was sortable.
Also refactors to use mergeprops.

[Finishes #97658788]
  • Loading branch information
Caroline Taymor and Kenny Wang committed Jul 16, 2015
1 parent 537f477 commit c3b2fa0
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 41 deletions.
80 changes: 43 additions & 37 deletions spec/pivotal-ui-react/sortable-table/sortable-table_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,30 @@ describe('SortableTable', function() {
const data = [
{
instances: '1',
bar: '9',
title: 'foo',
unsortable: '14'
},
{
instances: '3',
bar: '7',
title: 'sup',
unsortable: '22'
},
{
title: 'yee',
instances: '2',
bar: '8',
unsortable: '1'
}
];

function renderSortableTable(data, props = {}) {
clickSpy = jasmine.createSpy('click');
headers = [
<TableHeader sortable={true} onClick={clickSpy}>Title</TableHeader>,
<TableHeader sortable={true}>Instances</TableHeader>,
<TableHeader>Title</TableHeader>,
<TableHeader onClick={clickSpy} sortable={true}>Instances</TableHeader>,
<TableHeader sortable={true}>Bar</TableHeader>,
<TableHeader>Unsortable</TableHeader>
];

Expand All @@ -37,6 +41,7 @@ describe('SortableTable', function() {
<TableRow key={key}>
<TableCell>{datum.title}</TableCell>
<TableCell>{datum.instances}</TableCell>
<TableCell>{datum.bar}</TableCell>
<TableCell>{datum.unsortable}</TableCell>
</TableRow>
);
Expand All @@ -56,7 +61,7 @@ describe('SortableTable', function() {
});

it('adds the class "sortable" on all sortable columns', function() {
expect('th:contains("Title")').toHaveClass('sortable');
expect('th:contains("Title")').not.toHaveClass('sortable');
expect('th:contains("Instances")').toHaveClass('sortable');
expect('th:contains("Unsortable")').not.toHaveClass('sortable');
});
Expand All @@ -70,79 +75,79 @@ describe('SortableTable', function() {
expect('table.table-sortable').toHaveCss({opacity: '0.5'});
});

it('sorts table rows by the first column in ascending order by default', function() {
expect('th:contains("Title")').toHaveClass('sorted-asc');
it('sorts table rows by the first sortable column in ascending order by default', function() {
expect('th:contains("Instances")').toHaveClass('sorted-asc');

expect('tbody tr:nth-of-type(1) > td:eq(0)').toContainText('foo');
expect('tbody tr:nth-of-type(2) > td:eq(0)').toContainText('sup');
expect('tbody tr:nth-of-type(3) > td:eq(0)').toContainText('yee');
expect('tbody tr:nth-of-type(2) > td:eq(0)').toContainText('yee');
expect('tbody tr:nth-of-type(3) > td:eq(0)').toContainText('sup');

expect('tbody tr:nth-of-type(1) > td:eq(1)').toContainText('1');
expect('tbody tr:nth-of-type(2) > td:eq(1)').toContainText('3');
expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('2');
expect('tbody tr:nth-of-type(2) > td:eq(1)').toContainText('2');
expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('3');
});

describe('clicking on the already asc-sorted column that has an existing onClick function', function() {
beforeEach(function() {
$('th:contains("Title")').simulate('click');
$('th:contains("Instances")').simulate('click');
});

it('calls the onClick function', function() {
expect(clickSpy).toHaveBeenCalled();
});

it('reverses the sort order', function() {
expect('th:contains("Title")').toHaveClass('sorted-desc');
expect('th:contains("Instances")').toHaveClass('sorted-desc');

expect('tbody tr:nth-of-type(1) > td:eq(0)').toContainText('yee');
expect('tbody tr:nth-of-type(2) > td:eq(0)').toContainText('sup');
expect('tbody tr:nth-of-type(1) > td:eq(0)').toContainText('sup');
expect('tbody tr:nth-of-type(2) > td:eq(0)').toContainText('yee');
expect('tbody tr:nth-of-type(3) > td:eq(0)').toContainText('foo');

expect('tbody tr:nth-of-type(1) > td:eq(1)').toContainText('2');
expect('tbody tr:nth-of-type(2) > td:eq(1)').toContainText('3');
expect('tbody tr:nth-of-type(1) > td:eq(1)').toContainText('3');
expect('tbody tr:nth-of-type(2) > td:eq(1)').toContainText('2');
expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('1');
});

describe('clicking on the already desc-sorted column', function() {
beforeEach(function() {
clickSpy.calls.reset();
$('th:contains("Title")').simulate('click');
$('th:contains("Instances")').simulate('click');
});

it('calls the onClick function', function() {
expect(clickSpy).toHaveBeenCalled();
});

it('reverses the sort order', function() {
expect('th:contains("Title")').toHaveClass('sorted-asc');
expect('th:contains("Instances")').toHaveClass('sorted-asc');

expect('tbody tr:nth-of-type(1) > td:eq(0)').toContainText('foo');
expect('tbody tr:nth-of-type(2) > td:eq(0)').toContainText('sup');
expect('tbody tr:nth-of-type(3) > td:eq(0)').toContainText('yee');
expect('tbody tr:nth-of-type(2) > td:eq(0)').toContainText('yee');
expect('tbody tr:nth-of-type(3) > td:eq(0)').toContainText('sup');

expect('tbody tr:nth-of-type(1) > td:eq(1)').toContainText('1');
expect('tbody tr:nth-of-type(2) > td:eq(1)').toContainText('3');
expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('2');
expect('tbody tr:nth-of-type(2) > td:eq(1)').toContainText('2');
expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('3');
});
});
});

describe('clicking on a sortable column', function() {
beforeEach(function() {
$('th:contains("Instances")').simulate('click');
$('th:contains("Bar")').simulate('click');
});

it('sorts table rows by that column', function() {
expect('th:contains("Instances")').toHaveClass('sorted-asc');
expect('th:contains("Bar")').toHaveClass('sorted-asc');
expect('th:contains("Title")').not.toHaveClass('sorted-asc');

expect('tbody tr:nth-of-type(1) > td:eq(0)').toContainText('foo');
expect('tbody tr:nth-of-type(1) > td:eq(0)').toContainText('sup');
expect('tbody tr:nth-of-type(2) > td:eq(0)').toContainText('yee');
expect('tbody tr:nth-of-type(3) > td:eq(0)').toContainText('sup');
expect('tbody tr:nth-of-type(3) > td:eq(0)').toContainText('foo');

expect('tbody tr:nth-of-type(1) > td:eq(1)').toContainText('1');
expect('tbody tr:nth-of-type(2) > td:eq(1)').toContainText('2');
expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('3');
expect('tbody tr:nth-of-type(1) > td:eq(2)').toContainText('7');
expect('tbody tr:nth-of-type(2) > td:eq(2)').toContainText('8');
expect('tbody tr:nth-of-type(3) > td:eq(2)').toContainText('9');
});
});

Expand All @@ -153,32 +158,33 @@ describe('SortableTable', function() {

it('does not change the sort', function() {
expect('th:contains("Unsortable")').not.toHaveClass('sorted-asc');
expect('th:contains("Title")').toHaveClass('sorted-asc');
expect('th:contains("Instances")').toHaveClass('sorted-asc');

expect('tbody tr:nth-of-type(1) > td:eq(0)').toContainText('foo');
expect('tbody tr:nth-of-type(2) > td:eq(0)').toContainText('sup');
expect('tbody tr:nth-of-type(3) > td:eq(0)').toContainText('yee');
expect('tbody tr:nth-of-type(2) > td:eq(0)').toContainText('yee');
expect('tbody tr:nth-of-type(3) > td:eq(0)').toContainText('sup');

expect('tbody tr:nth-of-type(1) > td:eq(1)').toContainText('1');
expect('tbody tr:nth-of-type(2) > td:eq(1)').toContainText('3');
expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('2');
expect('tbody tr:nth-of-type(2) > td:eq(1)').toContainText('2');
expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('3');
});
});

describe('when the rows change', function() {
beforeEach(function() {
var newData = data.concat({
title: 'new title',
instances: '3',
unsortable: '1'
instances: '1.5',
unsortable: '1',
bar: '123'
});
renderSortableTable(newData);
});

it('shows the new rows in the correct sort order', function() {
expect('tbody tr').toHaveLength(4);
var titles = $('tbody tr > td:first-child').map(function() {return $(this).text(); }).toArray();
expect(titles).toEqual(['foo', 'new title', 'sup', 'yee']);
var instances = $('tbody tr > td:nth-of-type(2)').map(function() {return $(this).text(); }).toArray();
expect(instances).toEqual(['1', '1.5', '2', '3']);
});
});
});
Expand Down
12 changes: 9 additions & 3 deletions src/pivotal-ui-react/sortable-table/sortable-table.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const classnames = require('classnames');
const React = require('react');
const sortBy = require('lodash.sortby');
import {mergeProps} from '../../../src/pivotal-ui-react/helpers/helpers';


const types = React.PropTypes;

Expand Down Expand Up @@ -96,7 +98,11 @@ export const SortableTable = React.createClass({
},

getInitialState() {
return {sortColumn: 0, sortAscending: true};
const sortCol = this.props.headers.findIndex((header) => {
return header.props.sortable;
});
// If none of the columns are sortable we default to the 0th column
return {sortColumn: sortCol === -1 ? 0 : sortCol, sortAscending: true};
},

setSortedColumn(sortColumn, sortAscending) {
Expand Down Expand Up @@ -138,11 +144,11 @@ export const SortableTable = React.createClass({
},

render() {
let {className, style, id} = this.props;
const props = mergeProps(this.props, {className: ['table', 'table-sortable']});
let rows = this.sortedRows();

return (
<table className={classnames('table', 'table-sortable', className)} id={id} style={style} >
<table {...props} >
<thead>
<tr>{this.renderHeaders()}</tr>
</thead>
Expand Down
3 changes: 2 additions & 1 deletion src/pivotal-ui/javascripts/components.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,5 @@ module.exports = {
BackToTop: require('pui-react-back-to-top').BackToTop,

PortalSource: require('pui-react-portals').PortalSource,
PortalDestination: require('pui-react-portals').PortalDestination, ...sortableTable};
PortalDestination: require('pui-react-portals').PortalDestination,
...sortableTable};

0 comments on commit c3b2fa0

Please sign in to comment.