From 537f47707390ff26e67db3b1065f1008faabc9d4 Mon Sep 17 00:00:00 2001 From: "Caroline Taymor, Matt Royal and Ryan Dy" Date: Tue, 14 Jul 2015 15:00:28 -0700 Subject: [PATCH 1/2] fix(sortable-table): refactor sortable-table to rerender with data change New interface for sortable-table, which also fixes the bug where the table didn't rerender when the data changed. [Finishes #97658788] BREAKING CHANGE: (javascript) React component has new wrapping components for the header and rows. --- .../sortable-table/sortable-table_spec.js | 266 ++++++++++++++---- .../sortable-table/sortable-table.js | 195 ++++++------- src/pivotal-ui/components/tables/tables.scss | 67 ++--- src/pivotal-ui/javascripts/components.js | 7 +- 4 files changed, 345 insertions(+), 190 deletions(-) diff --git a/spec/pivotal-ui-react/sortable-table/sortable-table_spec.js b/spec/pivotal-ui-react/sortable-table/sortable-table_spec.js index 1adcfa6ce..9e94b2c13 100644 --- a/spec/pivotal-ui-react/sortable-table/sortable-table_spec.js +++ b/spec/pivotal-ui-react/sortable-table/sortable-table_spec.js @@ -1,46 +1,54 @@ require('../spec_helper'); +import {SortableTable, TableCell, TableHeader, TableRow} from '../../../src/pivotal-ui-react/sortable-table/sortable-table'; + describe('SortableTable', function() { - var SortableTable, data, columns; - beforeEach(function() { - SortableTable = require('../../../src/pivotal-ui-react/sortable-table/sortable-table').SortableTable; - columns = [ - { - name: 'title', - title: 'Title', - sortable: true - }, - { - name: 'instances', - title: 'Instances', - sortable: true, - align: 'center' - }, - { - name: 'unsortable', - title: 'Unsortable', - sortable: false, - align: 'right' - } - ]; - data = [ - { - instances: '1', - title: 'foo', - unsortable: '14' - }, - { - instances: '3', - title: 'sup', - unsortable: '22' - }, - { - title: 'yee', - instances: '2', - unsortable: '1' - } + var headers, clickSpy; + + const data = [ + { + instances: '1', + title: 'foo', + unsortable: '14' + }, + { + instances: '3', + title: 'sup', + unsortable: '22' + }, + { + title: 'yee', + instances: '2', + unsortable: '1' + } + ]; + + function renderSortableTable(data, props = {}) { + clickSpy = jasmine.createSpy('click'); + headers = [ + Title, + Instances, + Unsortable ]; - React.render(, root); + React.render(( + + {data.map(function(datum, key) { + return ( + + {datum.title} + {datum.instances} + {datum.unsortable} + + ); + })} + + ), + root + ); + } + + beforeEach(function() { + renderSortableTable(data); }); afterEach(function() { @@ -53,18 +61,13 @@ describe('SortableTable', function() { expect('th:contains("Unsortable")').not.toHaveClass('sortable'); }); - it('adds the additional classes specified in the "classes" property', function() { - React.render(, root); - + it('adds the additional classes, id and styles to the table', function() { + renderSortableTable(data, {className: ['table-light'], id: 'table-id', style: {opacity: '0.5'}}); expect('table.table-sortable').toHaveClass('table'); expect('table.table-sortable').toHaveClass('table-sortable'); expect('table.table-sortable').toHaveClass('table-light'); - }); - - it('column data is aligned to the specified margin', function() { - expect('tbody tr:nth-of-type(1) > td:eq(0)').not.toHaveClass('txt-l'); - expect('tbody tr:nth-of-type(2) > td:eq(1)').toHaveClass('txt-c'); - expect('tbody tr:nth-of-type(1) > td:eq(2)').toHaveClass('txt-r'); + expect('table.table-sortable').toHaveProp('id', 'table-id'); + expect('table.table-sortable').toHaveCss({opacity: '0.5'}); }); it('sorts table rows by the first column in ascending order by default', function() { @@ -79,11 +82,15 @@ describe('SortableTable', function() { expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('2'); }); - describe('clicking on the already asc-sorted column', function() { + describe('clicking on the already asc-sorted column that has an existing onClick function', function() { beforeEach(function() { $('th:contains("Title")').simulate('click'); }); + it('calls the onClick function', function() { + expect(clickSpy).toHaveBeenCalled(); + }); + it('reverses the sort order', function() { expect('th:contains("Title")').toHaveClass('sorted-desc'); @@ -98,9 +105,14 @@ describe('SortableTable', function() { describe('clicking on the already desc-sorted column', function() { beforeEach(function() { + clickSpy.calls.reset(); $('th:contains("Title")').simulate('click'); }); + it('calls the onClick function', function() { + expect(clickSpy).toHaveBeenCalled(); + }); + it('reverses the sort order', function() { expect('th:contains("Title")').toHaveClass('sorted-asc'); @@ -152,5 +164,161 @@ describe('SortableTable', function() { expect('tbody tr:nth-of-type(3) > td:eq(1)').toContainText('2'); }); }); + + describe('when the rows change', function() { + beforeEach(function() { + var newData = data.concat({ + title: 'new title', + instances: '3', + unsortable: '1' + }); + 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']); + }); + }); }); +describe('TableHeader', function() { + function renderTableHeader({children, ...props}) { + return React.render(( + + + + + {children} + + + +
+ ), root + ); + + } + + it('contains the given children', function() { + renderTableHeader({children: (

)}); + expect('th').toExist(); + expect('th > p#header-id').toExist(); + }); + + describe('when the header is sortable', function() { + const props = { + sortable: true, + id: 'header-id', + className: 'header-light', + style: {opacity: '0.5'} + }; + + it('adds the additional classes, id and styles to the th', function() { + renderTableHeader(props); + expect('th').toHaveClass('sortable'); + expect('th').toHaveClass('header-light'); + expect('th').toHaveProp('id', 'header-id'); + expect('th').toHaveCss({opacity: '0.5'}); + }); + + describe('when there is an onSortableTableHeaderClick provided', function() { + var onSortableTableHeaderClickSpy; + beforeEach(function() { + onSortableTableHeaderClickSpy = jasmine.createSpy('onSortableTableHeaderClick'); + renderTableHeader({onSortableTableHeaderClick: onSortableTableHeaderClickSpy, ...props}); + }); + + describe('when clicking on the table header', function() { + it('calls the callback', function() { + $('th').simulate('click'); + expect(onSortableTableHeaderClickSpy).toHaveBeenCalled(); + }); + }); + }); + }); + + describe('when the header is not sortable', function() { + it('adds the additional classes, id and styles to the th', function() { + renderTableHeader({ + sortable: false, + id: 'header-id', + className: 'header-light', + style: {opacity: '0.5'} + }); + expect('th').not.toHaveClass('sortable'); + expect('th').toHaveClass('header-light'); + expect('th').toHaveProp('id', 'header-id'); + expect('th').toHaveCss({opacity: '0.5'}); + }); + }); +}); + +describe('TableRow', function() { + function renderTableRow({children=(), ...props}) { + return React.render(( + + + + {children} + + +
+ ), root + ); + + } + it('contains the given children', function() { + renderTableRow({children: ()}); + expect('tr').toExist(); + expect('tr > td#cell-id').toExist(); + }); + + + it('adds the additional classes, id and styles to the th', function() { + renderTableRow({ + id: 'row-id', + className: 'row-light', + style: {opacity: '0.5'} + }); + expect('tr').toHaveClass('row-light'); + expect('tr').toHaveProp('id', 'row-id'); + expect('tr').toHaveCss({opacity: '0.5'}); + }); +}); + +describe('TableCell', function() { + function renderTableCell({children, ...props}) { + return React.render(( + + + + + {children} + + + +
+ ), root + ); + + } + + it('contains the given children', function() { + renderTableCell({children: (

This is my text

)}); + expect('td').toExist(); + expect('td > p').toExist(); + expect('td > p').toContainText('This is my text'); + }); + + it('adds the additional classes, id and styles to the th', function() { + renderTableCell({ + id: 'cell-id', + className: 'cell-light', + style: {opacity: '0.5'} + }); + expect('td').toHaveClass('cell-light'); + expect('td').toHaveProp('id', 'cell-id'); + expect('td').toHaveCss({opacity: '0.5'}); + }); +}); diff --git a/src/pivotal-ui-react/sortable-table/sortable-table.js b/src/pivotal-ui-react/sortable-table/sortable-table.js index a40e04ca2..162c4d8bd 100644 --- a/src/pivotal-ui-react/sortable-table/sortable-table.js +++ b/src/pivotal-ui-react/sortable-table/sortable-table.js @@ -1,42 +1,58 @@ -var classnames = require('classnames'); -var React = require('react'); -var sortBy = require('lodash.sortby'); +const classnames = require('classnames'); +const React = require('react'); +const sortBy = require('lodash.sortby'); -var types = React.PropTypes; +const types = React.PropTypes; -var TableHeader = React.createClass({ - handleClick() { - if (this.props.sortable) { - this.props.onSortableTableHeaderClick(this); - } +/** + * @component TableHeader + * @description Wrapper for a th + * + * @property `sortable` {boolean} (defaults to false) indicates whether the table can be sorted by this column; + * + * @see [Pivotal UI React](http://styleguide.pivotal.io/react_beta.html#table_sortable_react) + */ +export const TableHeader = React.createClass({ + propTypes: { + onClick: types.func, + onSortableTableHeaderClick: types.func + }, + + handleClick(...args) { + var {sortable, onClick, onSortableTableHeaderClick} = this.props; + if (sortable) onSortableTableHeaderClick(...args); + if (onClick) onClick(...args); }, render() { - var {children, sortable, sortState: {column, order}, name} = this.props; + const {sortable, className, ...props} = this.props; + const classes = classnames({'sortable': sortable}, className); - var sortClass = classnames({ - 'sortable': sortable, - 'sorted-none': sortable && column !== name, - 'sorted-asc': sortable && column === name && order === 'asc', - 'sorted-desc': sortable && column === name && order === 'desc' - }); + return ; + } +}); - return {children}; +/** + * @component TableCell + * @description Wrapper for a td + * + * @see [Pivotal UI React](http://styleguide.pivotal.io/react_beta.html#table_sortable_react) + */ +export const TableCell = React.createClass({ + render() { + return ; } }); -const ALIGNMENT = { - left: 'txt-l', - center: 'txt-c', - right: 'txt-r' -}; -var TableRow = React.createClass({ +/** + * @component TableRow + * @description Wrapper for a tr + * + * @see [Pivotal UI React](http://styleguide.pivotal.io/react_beta.html#table_sortable_react) + */ +export const TableRow = React.createClass({ render() { - var {data, alignment, columnNames} = this.props; - var tableCells = columnNames.map(function(name, i) { - return {data[name]}; - }); - return {tableCells}; + return ; } }); @@ -44,100 +60,91 @@ var TableRow = React.createClass({ * @component SortableTable * @description A table that can be sorted by column * - * @property columns {Array} A list of column configuration parameters: - * `name` is the `data` key associated with the column; - * `title` is the header text for the column; - * `sortable` (defaults to false) indicates whether the table can be sorted by this column; and - * `align` (defaults to left) sets the text alignment for cells in this column - * @property data {Array} The data to be displayed in the table - * @property classes {Array} Class names to add to the table + * @property `headers` {Array} A list of `TableHeader` components * * @example ```js - * var SortableTable = require('pui-react-sortable-table').SortableTable; + * var {SortableTable, TableHeader, TableRow, TableCell} = require('pui-react-sortable-table'); * var MyComponent = React.createClass({ * render() { - * var columns = [ - * {name: 'c1', title: 'Column 1', sortable: true}, - * {name: 'c2', title: 'Column 2'} - * ]; + * var headers = [ + * c1, + * c2, + * ]; * var data = [ * {c1: 'yes', c2: 'foo'}, * {c1: 'no', c2: 'bar'} * ]; - * return ; + * return + * {sortTableData.map(function(datum, key) { + * return ( + * + * {datum.c1} + * {datum.c2} + * + * ); + * })} + * ; * } * }); * ``` * * @see [Pivotal UI React](http://styleguide.pivotal.io/react.html#table_sortable_react) */ -var SortableTable = React.createClass({ +export const SortableTable = React.createClass({ propTypes: { - classes: types.arrayOf(types.string), - columns: types.arrayOf(types.shape({ - name: types.string.isRequired, - title: types.string.isRequired, - align: types.oneOf(['left', 'center', 'right']), - sortable: types.bool - })), - data: types.arrayOf(types.object) + headers: types.arrayOf(types.element) }, getInitialState() { - var {columns, data} = this.props; - var column = columns[0].name; - return { - sort: { - column, - order: 'asc' - }, - data: sortBy(data, column) - }; + return {sortColumn: 0, sortAscending: true}; }, - sortData(header) { - var {data: oldData, sort: {column: oldSortColumn, order: oldSortOrder}} = this.state; - var newSortColumn = header.props.name; - var newData, newSortOrder; - - if (oldSortColumn !== newSortColumn) { - newSortOrder = 'asc'; - newData = sortBy(oldData, newSortColumn); - } else { - newSortOrder = oldSortOrder === 'asc' ? 'desc' : 'asc'; - newData = oldData.reverse(); - } + setSortedColumn(sortColumn, sortAscending) { + this.setState({sortColumn, sortAscending}); + }, - this.setState({ - sort: { - column: newSortColumn, - order: newSortOrder - }, - data: newData + headerClassesWithSortDirection({headerClassName, isSortColumn}) { + return classnames(headerClassName, { + 'sorted-asc': isSortColumn && this.state.sortAscending, + 'sorted-desc': isSortColumn && !this.state.sortAscending }); }, - render() { - var {columns, classes} = this.props; - - var headings = columns.map(function(column) { - return ( - - {column.title} - - ); - }, this); + sortedRows() { + const {sortColumn, sortAscending} = this.state; + const sortedRows = sortBy(this.props.children, (row) => { + const cellForSorting = row.props.children[sortColumn]; + return cellForSorting.props.children; + }); + return sortAscending ? sortedRows : sortedRows.reverse(); + }, - var rows = this.state.data.map(function(datum, i) { - return ( c.name)} alignment={columns.map(c => c.align)}/>); + renderHeaders() { + var {headers} = this.props; + const {sortColumn, sortAscending} = this.state; + return headers.map((header, index) => { + const isSortColumn = (sortColumn === index); + return React.cloneElement(header, { + key: index, + className: this.headerClassesWithSortDirection( + { + headerClassName: header.props.className, + isSortColumn + } + ), + onSortableTableHeaderClick: () => this.setSortedColumn(index, isSortColumn ? !sortAscending : true) + }); }); + }, + + render() { + let {className, style, id} = this.props; + let rows = this.sortedRows(); return ( - +
- - {headings} - + {this.renderHeaders()} {rows} @@ -146,5 +153,3 @@ var SortableTable = React.createClass({ ); } }); - -module.exports = {SortableTable}; diff --git a/src/pivotal-ui/components/tables/tables.scss b/src/pivotal-ui/components/tables/tables.scss index 37d232f1f..425ffcbed 100644 --- a/src/pivotal-ui/components/tables/tables.scss +++ b/src/pivotal-ui/components/tables/tables.scss @@ -335,51 +335,21 @@ functioning sort. The `SortableTable` expects the following properties: -Property | Type | Description -----------| ---------------------------------| -------------------------------------------------------------------------- -`data` | Array of JS objects | The collection of data to be displayed -`columns` | Array of column objects | Informs the table how to render columns as well as which columns should be sortable -`classes` | Array of strings | A list of class names to be added to the table element +Property | Required? | Type | Description +-----------| ----------| ---------------------------------| -------------------------------------------------------------------------- +`headers` | **yes** | Array of TableHeader components | The headers to display in the desired order +The `TableHeader` objects should have the following structure: -The column objects should have the following structure: +Property | Required? | Type | Description +-----------| ----------|------------------| -------------------------------------------------------------------------- +`sortable` | no | Boolean | Is this column sortable? Defaults to false -Property | Required? | Type | Description ------------| ----------|------------------------------| -------------------------------------------------------------------------- -`name` | **yes** | String | Name of the property on a data object (how to lookup the value for this column) -`title` | **yes** | String | Name to display on the table header -`sortable` | no | Boolean | Is this column sortable? Defaults to false -`align` | no | 'center', 'left', or 'right' | How should text be aligned within this column? Defaults to left If a column is marked as being sortable, it will attempt to sort the values as strings. -```js_example -var sortTableCols = [ - { - name: 'name', - title: 'Name', - sortable: true - }, - { - name: 'instances', - title: 'Instances', - sortable: true, - align: 'center' - }, - { - name: 'cpu', - title: 'CPU', - sortable: true, - align: 'right' - }, - { - name: 'synergy', - title: 'Synergy', - align: 'left' - } -] - +```jsx_example var sortTableData = [ { instances: '1', @@ -411,9 +381,24 @@ var sortTableData = [ ```react_example + headers={[ + Name, + Instances, + CPU, + Synergy, + ]} + className="table-light"> + {sortTableData.map(function(datum, key) { + return ( + + {datum.name} + {datum.instances} + {datum.cpu} + {datum.synergy} + + ); + })} + ``` */ diff --git a/src/pivotal-ui/javascripts/components.js b/src/pivotal-ui/javascripts/components.js index a599ca0ec..82011b5f1 100644 --- a/src/pivotal-ui/javascripts/components.js +++ b/src/pivotal-ui/javascripts/components.js @@ -1,7 +1,5 @@ - +var sortableTable = require('pui-react-sortable-table'); module.exports = { - SortableTable: require('pui-react-sortable-table').SortableTable, - DefaultH1: require('pui-react-typography').DefaultH1, DefaultH2: require('pui-react-typography').DefaultH2, DefaultH3: require('pui-react-typography').DefaultH3, @@ -120,5 +118,4 @@ module.exports = { BackToTop: require('pui-react-back-to-top').BackToTop, PortalSource: require('pui-react-portals').PortalSource, - PortalDestination: require('pui-react-portals').PortalDestination -}; + PortalDestination: require('pui-react-portals').PortalDestination, ...sortableTable}; \ No newline at end of file From c3b2fa0c029e5002e16eb8345e005c01327fd3b8 Mon Sep 17 00:00:00 2001 From: Caroline Taymor and Kenny Wang Date: Wed, 15 Jul 2015 17:57:39 -0700 Subject: [PATCH 2/2] fix(sortable-table): sorts on first sortable column instead of on the first column regardless of whether it was sortable. Also refactors to use mergeprops. [Finishes #97658788] --- .../sortable-table/sortable-table_spec.js | 80 ++++++++++--------- .../sortable-table/sortable-table.js | 12 ++- src/pivotal-ui/javascripts/components.js | 3 +- 3 files changed, 54 insertions(+), 41 deletions(-) diff --git a/spec/pivotal-ui-react/sortable-table/sortable-table_spec.js b/spec/pivotal-ui-react/sortable-table/sortable-table_spec.js index 9e94b2c13..465c653f7 100644 --- a/spec/pivotal-ui-react/sortable-table/sortable-table_spec.js +++ b/spec/pivotal-ui-react/sortable-table/sortable-table_spec.js @@ -7,17 +7,20 @@ 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' } ]; @@ -25,8 +28,9 @@ describe('SortableTable', function() { function renderSortableTable(data, props = {}) { clickSpy = jasmine.createSpy('click'); headers = [ - Title, - Instances, + Title, + Instances, + Bar, Unsortable ]; @@ -37,6 +41,7 @@ describe('SortableTable', function() { {datum.title} {datum.instances} + {datum.bar} {datum.unsortable} ); @@ -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'); }); @@ -70,21 +75,21 @@ 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() { @@ -92,21 +97,21 @@ describe('SortableTable', function() { }); 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() { @@ -114,35 +119,35 @@ describe('SortableTable', function() { }); 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'); }); }); @@ -153,15 +158,15 @@ 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'); }); }); @@ -169,16 +174,17 @@ describe('SortableTable', 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']); }); }); }); diff --git a/src/pivotal-ui-react/sortable-table/sortable-table.js b/src/pivotal-ui-react/sortable-table/sortable-table.js index 162c4d8bd..4865aed7a 100644 --- a/src/pivotal-ui-react/sortable-table/sortable-table.js +++ b/src/pivotal-ui-react/sortable-table/sortable-table.js @@ -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; @@ -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) { @@ -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 ( -
+
{this.renderHeaders()} diff --git a/src/pivotal-ui/javascripts/components.js b/src/pivotal-ui/javascripts/components.js index 82011b5f1..027e818b3 100644 --- a/src/pivotal-ui/javascripts/components.js +++ b/src/pivotal-ui/javascripts/components.js @@ -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}; \ No newline at end of file + PortalDestination: require('pui-react-portals').PortalDestination, + ...sortableTable};