Skip to content

Commit f87ddf4

Browse files
glittersharkglittershark
committed
Merge pull request glittershark#242 from cklab/master
issue glittershark#237 - column attribute ignored when <Td> has children
2 parents 403b4e1 + 70b4121 commit f87ddf4

File tree

5 files changed

+217
-74
lines changed

5 files changed

+217
-74
lines changed

‎build/reactable.js‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -727,16 +727,31 @@ window.ReactDOM["default"] = window.ReactDOM;
727727
// Can't use React.Children.map since that doesn't return a proper array
728728
var columns = [];
729729
_react['default'].Children.forEach(component.props.children, function (th) {
730-
if (typeof th.props.children === 'string') {
731-
columns.push(th.props.children);
732-
} else if (typeof th.props.column === 'string') {
733-
columns.push({
734-
key: th.props.column,
735-
label: th.props.children,
736-
props: (0, _libFilter_props_from.filterPropsFrom)(th.props)
737-
});
738-
} else {
730+
var column = {};
731+
if (typeof th.props !== 'undefined') {
732+
column.props = (0, _libFilter_props_from.filterPropsFrom)(th.props);
733+
734+
// use the content as the label & key
735+
if (typeof th.props.children !== 'undefined') {
736+
column.label = th.props.children;
737+
column.key = column.label;
738+
}
739+
740+
// the key in the column attribute supersedes the one defined previously
741+
if (typeof th.props.column === 'string') {
742+
column.key = th.props.column;
743+
744+
// in case we don't have a label yet
745+
if (typeof column.label === 'undefined') {
746+
column.label = column.key;
747+
}
748+
}
749+
}
750+
751+
if (typeof column.key === 'undefined') {
739752
throw new TypeError('<th> must have either a "column" property or a string ' + 'child');
753+
} else {
754+
columns.push(column);
740755
}
741756
});
742757

‎build/tests/reactable_test.js‎

Lines changed: 83 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -765,39 +765,74 @@
765765
});
766766

767767
describe('specifying columns using a <Thead>', function () {
768-
before(function () {
769-
ReactDOM.render(React.createElement(
770-
Reactable.Table,
771-
{ id: 'table', data: [{ Name: Reactable.unsafe('<span id="griffins-name">Griffin Smith</span>'), Age: '18' }, { Age: '28', Position: Reactable.unsafe('<span id="who-knows-job">Developer</span>') }, { Age: '23', Name: Reactable.unsafe('<span id="lees-name">Lee Salminen</span>') }] },
772-
React.createElement(
773-
Reactable.Thead,
774-
null,
768+
describe('and an element for the column title', function () {
769+
before(function () {
770+
ReactDOM.render(React.createElement(
771+
Reactable.Table,
772+
{ id: 'table', data: [{ Name: Reactable.unsafe('<span id="griffins-name">Griffin Smith</span>'), Age: '18' }, { Age: '28', Position: Reactable.unsafe('<span id="who-knows-job">Developer</span>') }, { Age: '23', Name: Reactable.unsafe('<span id="lees-name">Lee Salminen</span>') }] },
775773
React.createElement(
776-
Reactable.Th,
777-
{ column: 'Name', id: 'my-name' },
774+
Reactable.Thead,
775+
null,
778776
React.createElement(
779-
'strong',
780-
null,
781-
'name'
777+
Reactable.Th,
778+
{ column: 'Name', id: 'my-name' },
779+
React.createElement(
780+
'strong',
781+
null,
782+
'name'
783+
)
782784
)
783785
)
784-
)
785-
), ReactableTestUtils.testNode());
786-
});
786+
), ReactableTestUtils.testNode());
787+
});
787788

788-
after(ReactableTestUtils.resetTestEnvironment);
789+
after(ReactableTestUtils.resetTestEnvironment);
789790

790-
it('renders only the columns in the Thead', function () {
791-
expect($('#table tbody tr:first td')).to.exist;
792-
expect($('#table thead tr:first th')).to.exist;
793-
});
791+
it('renders only the columns in the Thead', function () {
792+
expect($('#table tbody tr:first td')).to.exist;
793+
expect($('#table thead tr:first th')).to.exist;
794+
});
795+
796+
it('renders the contents of the Th', function () {
797+
expect($('#table>thead>tr>th>strong')).to.exist;
798+
});
794799

795-
it('renders the contents of the Th', function () {
796-
expect($('#table>thead>tr>th>strong')).to.exist;
800+
it('passes through the properties of the Th', function () {
801+
expect($('#table>thead>tr>th')).to.have.id('my-name');
802+
});
797803
});
798804

799-
it('passes through the properties of the Th', function () {
800-
expect($('#table>thead>tr>th')).to.have.id('my-name');
805+
describe('and a string for the column title', function () {
806+
before(function () {
807+
ReactDOM.render(React.createElement(
808+
Reactable.Table,
809+
{ id: 'table', data: [{ Name: Reactable.unsafe('<span id="griffins-name">Griffin Smith</span>'), Age: '18' }, { Age: '28', Position: Reactable.unsafe('<span id="who-knows-job">Developer</span>') }, { Age: '23', Name: Reactable.unsafe('<span id="lees-name">Lee Salminen</span>') }] },
810+
React.createElement(
811+
Reactable.Thead,
812+
null,
813+
React.createElement(
814+
Reactable.Th,
815+
{ column: 'Name', id: 'my-name' },
816+
'name'
817+
)
818+
)
819+
), ReactableTestUtils.testNode());
820+
});
821+
822+
after(ReactableTestUtils.resetTestEnvironment);
823+
824+
it('renders only the columns in the Thead', function () {
825+
expect($('#table tbody tr:first td')).to.exist;
826+
expect($('#table thead tr:first th')).to.exist;
827+
});
828+
829+
it('renders the contents of the Th', function () {
830+
expect($('#table>thead>tr>th')).to.exist;
831+
});
832+
833+
it('passes through the properties of the Th', function () {
834+
expect($('#table>thead>tr>th')).to.have.id('my-name');
835+
});
801836
});
802837
});
803838

@@ -1080,6 +1115,30 @@
10801115
expect($('#table tbody.reactable-data tr').length).to.equal(9);
10811116
});
10821117
});
1118+
1119+
describe('onPageChange hook', function () {
1120+
var currentPage = undefined;
1121+
var callback = function callback(page) {
1122+
currentPage = page;
1123+
};
1124+
before(function () {
1125+
ReactDOM.render(React.createElement(Reactable.Table, { className: 'table', id: 'table', data: [{ 'Name': 'Griffin Smith', 'Age': '18' }, { 'Age': '23', 'Name': 'Lee Salminen' }, { 'Age': '28', 'Position': 'Developer' }, { 'Name': 'Griffin Smith', 'Age': '18' }, { 'Age': '23', 'Name': 'Test Person' }, { 'Name': 'Ian Zhang', 'Age': '28', 'Position': 'Developer' }, { 'Name': 'Griffin Smith', 'Age': '18', 'Position': 'Software Developer' }, { 'Age': '23', 'Name': 'Lee Salminen' }, { 'Age': '28', 'Position': 'Developer' }], itemsPerPage: 4, onPageChange: callback }), ReactableTestUtils.testNode());
1126+
});
1127+
1128+
after(ReactableTestUtils.resetTestEnvironment);
1129+
1130+
it('emits the number of the currently selected page (zero based) when onPageChange event is triggered', function () {
1131+
var page1 = $('#table tbody.reactable-pagination a.reactable-page-button')[0];
1132+
var page2 = $('#table tbody.reactable-pagination a.reactable-page-button')[1];
1133+
var page3 = $('#table tbody.reactable-pagination a.reactable-page-button')[2];
1134+
ReactTestUtils.Simulate.click(page2);
1135+
expect(currentPage).to.equal(1);
1136+
ReactTestUtils.Simulate.click(page1);
1137+
expect(currentPage).to.equal(0);
1138+
ReactTestUtils.Simulate.click(page3);
1139+
expect(currentPage).to.equal(2);
1140+
});
1141+
});
10831142
});
10841143

10851144
describe('sorting', function () {

‎lib/reactable/thead.js‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,31 @@ var Thead = (function (_React$Component) {
108108
// Can't use React.Children.map since that doesn't return a proper array
109109
var columns = [];
110110
_react2['default'].Children.forEach(component.props.children, function (th) {
111-
if (typeof th.props.children === 'string') {
112-
columns.push(th.props.children);
113-
} else if (typeof th.props.column === 'string') {
114-
columns.push({
115-
key: th.props.column,
116-
label: th.props.children,
117-
props: (0, _libFilter_props_from.filterPropsFrom)(th.props)
118-
});
119-
} else {
111+
var column = {};
112+
if (typeof th.props !== 'undefined') {
113+
column.props = (0, _libFilter_props_from.filterPropsFrom)(th.props);
114+
115+
// use the content as the label & key
116+
if (typeof th.props.children !== 'undefined') {
117+
column.label = th.props.children;
118+
column.key = column.label;
119+
}
120+
121+
// the key in the column attribute supersedes the one defined previously
122+
if (typeof th.props.column === 'string') {
123+
column.key = th.props.column;
124+
125+
// in case we don't have a label yet
126+
if (typeof column.label === 'undefined') {
127+
column.label = column.key;
128+
}
129+
}
130+
}
131+
132+
if (typeof column.key === 'undefined') {
120133
throw new TypeError('<th> must have either a "column" property or a string ' + 'child');
134+
} else {
135+
columns.push(column);
121136
}
122137
});
123138

‎src/reactable/thead.jsx‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,33 @@ export class Thead extends React.Component {
88
// Can't use React.Children.map since that doesn't return a proper array
99
let columns = [];
1010
React.Children.forEach(component.props.children, th => {
11-
if (typeof th.props.children === 'string') {
12-
columns.push(th.props.children);
13-
} else if (typeof th.props.column === 'string') {
14-
columns.push({
15-
key: th.props.column,
16-
label: th.props.children,
17-
props: filterPropsFrom(th.props)
18-
});
19-
} else {
11+
var column = {};
12+
if (typeof th.props !== 'undefined') {
13+
column.props = filterPropsFrom(th.props);
14+
15+
// use the content as the label & key
16+
if (typeof th.props.children !== 'undefined') {
17+
column.label = th.props.children;
18+
column.key = column.label;
19+
}
20+
21+
// the key in the column attribute supersedes the one defined previously
22+
if (typeof th.props.column === 'string') {
23+
column.key = th.props.column;
24+
25+
// in case we don't have a label yet
26+
if (typeof column.label === 'undefined') {
27+
column.label = column.key;
28+
}
29+
}
30+
}
31+
32+
if (typeof column.key === 'undefined') {
2033
throw new TypeError(
2134
'<th> must have either a "column" property or a string ' +
22-
'child');
35+
'child');
36+
} else {
37+
columns.push(column);
2338
}
2439
});
2540

‎tests/reactable_test.jsx‎

Lines changed: 61 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -561,39 +561,78 @@ describe('Reactable', function() {
561561
});
562562

563563
describe('specifying columns using a <Thead>', function() {
564-
before(function() {
565-
ReactDOM.render(
566-
<Reactable.Table id="table" data={[
564+
describe('and an element for the column title', function() {
565+
before(function() {
566+
ReactDOM.render(
567+
<Reactable.Table id="table" data={[
567568
{ Name: Reactable.unsafe('<span id="griffins-name">Griffin Smith</span>'), Age: '18'},
568569
{ Age: '28', Position: Reactable.unsafe('<span id="who-knows-job">Developer</span>')},
569570
{ Age: '23', Name: Reactable.unsafe('<span id="lees-name">Lee Salminen</span>')},
570571
]}>
571-
<Reactable.Thead>
572-
<Reactable.Th column="Name" id="my-name">
573-
<strong>name</strong>
574-
</Reactable.Th>
575-
</Reactable.Thead>
576-
</Reactable.Table>,
577-
ReactableTestUtils.testNode()
578-
);
579-
});
572+
<Reactable.Thead>
573+
<Reactable.Th column="Name" id="my-name">
574+
<strong>name</strong>
575+
</Reactable.Th>
576+
</Reactable.Thead>
577+
</Reactable.Table>,
578+
ReactableTestUtils.testNode()
579+
);
580+
});
580581

581-
after(ReactableTestUtils.resetTestEnvironment);
582+
after(ReactableTestUtils.resetTestEnvironment);
582583

583-
it('renders only the columns in the Thead', function() {
584-
expect($('#table tbody tr:first td')).to.exist;
585-
expect($('#table thead tr:first th')).to.exist;
586-
});
584+
it('renders only the columns in the Thead', function() {
585+
expect($('#table tbody tr:first td')).to.exist;
586+
expect($('#table thead tr:first th')).to.exist;
587+
});
587588

588-
it('renders the contents of the Th', function() {
589-
expect($('#table>thead>tr>th>strong')).to.exist;
590-
});
589+
it('renders the contents of the Th', function() {
590+
expect($('#table>thead>tr>th>strong')).to.exist;
591+
});
592+
593+
it('passes through the properties of the Th', function() {
594+
expect($('#table>thead>tr>th')).to.have.id('my-name')
595+
});
591596

592-
it('passes through the properties of the Th', function() {
593-
expect($('#table>thead>tr>th')).to.have.id('my-name')
594597
});
598+
599+
describe('and a string for the column title', function() {
600+
before(function() {
601+
ReactDOM.render(
602+
<Reactable.Table id="table" data={[
603+
{ Name: Reactable.unsafe('<span id="griffins-name">Griffin Smith</span>'), Age: '18'},
604+
{ Age: '28', Position: Reactable.unsafe('<span id="who-knows-job">Developer</span>')},
605+
{ Age: '23', Name: Reactable.unsafe('<span id="lees-name">Lee Salminen</span>')},
606+
]}>
607+
<Reactable.Thead>
608+
<Reactable.Th column="Name" id="my-name">
609+
name
610+
</Reactable.Th>
611+
</Reactable.Thead>
612+
</Reactable.Table>,
613+
ReactableTestUtils.testNode()
614+
);
615+
});
616+
617+
after(ReactableTestUtils.resetTestEnvironment);
618+
619+
it('renders only the columns in the Thead', function() {
620+
expect($('#table tbody tr:first td')).to.exist;
621+
expect($('#table thead tr:first th')).to.exist;
622+
});
623+
624+
it('renders the contents of the Th', function() {
625+
expect($('#table>thead>tr>th')).to.exist;
626+
});
627+
628+
it('passes through the properties of the Th', function() {
629+
expect($('#table>thead>tr>th')).to.have.id('my-name')
630+
});
631+
632+
})
595633
});
596634

635+
597636
describe('unsafe() strings', function() {
598637
context('in the <Table> directly', function() {
599638
before(function() {

0 commit comments

Comments
 (0)