Skip to content

Commit

Permalink
fix(DataTable): update sort order to go ASC -> DESC -> NONE (carbon-d…
Browse files Browse the repository at this point in the history 
…esign-system#1535)

* fix(DataTable): update sort order to go ASC -> DESC -> NONE

* fix(data-table): update Rort -> Sort

Co-Authored-By: joshblack <[email protected]>

* fix(data-table): update Rort -> Sort

Co-Authored-By: joshblack <[email protected]>

* feat(data-table): add aria-sort property to TableHeader
  • Loading branch information
@joshblack
joshblack authored Nov 13, 2018
1 parent a72bc33 commit ee555b6
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 59 deletions.
26 changes: 20 additions & 6 deletions src/components/DataTable/TableHeader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,29 @@ const translationKeys = {
const translateWithId = (key, { sortDirection, isSortHeader, sortStates }) => {
if (key === translationKeys.iconDescription) {
if (isSortHeader) {
const order =
sortDirection === sortStates.DESC ? 'descending' : 'ascending';
return `Sort rows by this header in ${order} order`;
// When transitioning, we know that the sequence of states is as follows:
// NONE -> ASC -> DESC -> NONE
if (sortDirection === sortStates.NONE) {
return 'Sort rows by this header in ascending order';
}
if (sortDirection === sortStates.ASC) {
return 'Sort rows by this header in descending order';
}

return 'Unsort rows by this header';
}
return `Sort rows by this header in descending order`;
return 'Sort rows by this header in ascending order';
}

return '';
};

const sortDirections = {
[sortStates.NONE]: 'none',
[sortStates.ASC]: 'ascending',
[sortStates.DESC]: 'descending',
};

const TableHeader = ({
className: headerClassName,
children,
Expand All @@ -46,11 +59,12 @@ const TableHeader = ({
'bx--table-sort-v2--active':
isSortHeader && sortDirection !== sortStates.NONE,
'bx--table-sort-v2--ascending':
isSortHeader && sortDirection === sortStates.ASC,
isSortHeader && sortDirection === sortStates.DESC,
});
const ariaSort = !isSortHeader ? 'none' : sortDirections[sortDirection];

return (
<th scope={scope} className={headerClassName}>
<th scope={scope} className={headerClassName} aria-sort={ariaSort}>
<button className={className} onClick={onClick} {...rest}>
<span className="bx--table-header-label">{children}</span>
<Icon
Expand Down
6 changes: 3 additions & 3 deletions src/components/DataTable/__tests__/DataTable-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ describe('DataTable', () => {
expect(wrapper.state('rowIds')).toEqual(['a', 'b', 'c']);
});

it('should reset to DESC ordering when another header is clicked', () => {
it('should reset to ASC ordering when another header is clicked', () => {
const wrapper = mount(<DataTable {...mockProps} />);
const firstHeader = getHeaderAt(wrapper, 0);
const secondHeader = getHeaderAt(wrapper, 1);
Expand All @@ -158,10 +158,10 @@ describe('DataTable', () => {

firstHeader.simulate('click');
expect(wrapper.state('rowIds')).toEqual(['c', 'b', 'a']);
expect(wrapper.state('sortDirection')).toBe(sortStates.ASC);
expect(wrapper.state('sortDirection')).toBe(sortStates.DESC);

secondHeader.simulate('click');
expect(wrapper.state('sortDirection')).toBe(sortStates.DESC);
expect(wrapper.state('sortDirection')).toBe(sortStates.ASC);
});
});

Expand Down
54 changes: 30 additions & 24 deletions src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ exports[`DataTable selection should have select-all default to un-checked if no
translateWithId={[Function]}
>
<th
aria-sort="none"
scope="col"
>
<button
Expand All @@ -180,7 +181,7 @@ exports[`DataTable selection should have select-all default to un-checked if no
</span>
<Icon
className="bx--table-sort-v2__icon"
description="Sort rows by this header in descending order"
description="Sort rows by this header in ascending order"
fillRule="evenodd"
icon={
Object {
Expand Down Expand Up @@ -208,8 +209,8 @@ exports[`DataTable selection should have select-all default to un-checked if no
role="img"
>
<svg
alt="Sort rows by this header in descending order"
aria-label="Sort rows by this header in descending order"
alt="Sort rows by this header in ascending order"
aria-label="Sort rows by this header in ascending order"
className="bx--table-sort-v2__icon"
fillRule="evenodd"
height="5"
Expand All @@ -218,7 +219,7 @@ exports[`DataTable selection should have select-all default to un-checked if no
width="10"
>
<title>
Sort rows by this header in descending order
Sort rows by this header in ascending order
</title>
<path
d="M0 5L5 .002 10 5z"
Expand All @@ -239,6 +240,7 @@ exports[`DataTable selection should have select-all default to un-checked if no
translateWithId={[Function]}
>
<th
aria-sort="none"
scope="col"
>
<button
Expand All @@ -252,7 +254,7 @@ exports[`DataTable selection should have select-all default to un-checked if no
</span>
<Icon
className="bx--table-sort-v2__icon"
description="Sort rows by this header in descending order"
description="Sort rows by this header in ascending order"
fillRule="evenodd"
icon={
Object {
Expand Down Expand Up @@ -280,8 +282,8 @@ exports[`DataTable selection should have select-all default to un-checked if no
role="img"
>
<svg
alt="Sort rows by this header in descending order"
aria-label="Sort rows by this header in descending order"
alt="Sort rows by this header in ascending order"
aria-label="Sort rows by this header in ascending order"
className="bx--table-sort-v2__icon"
fillRule="evenodd"
height="5"
Expand All @@ -290,7 +292,7 @@ exports[`DataTable selection should have select-all default to un-checked if no
width="10"
>
<title>
Sort rows by this header in descending order
Sort rows by this header in ascending order
</title>
<path
d="M0 5L5 .002 10 5z"
Expand Down Expand Up @@ -634,6 +636,7 @@ exports[`DataTable selection should render 1`] = `
translateWithId={[Function]}
>
<th
aria-sort="none"
scope="col"
>
<button
Expand All @@ -647,7 +650,7 @@ exports[`DataTable selection should render 1`] = `
</span>
<Icon
className="bx--table-sort-v2__icon"
description="Sort rows by this header in descending order"
description="Sort rows by this header in ascending order"
fillRule="evenodd"
icon={
Object {
Expand Down Expand Up @@ -675,8 +678,8 @@ exports[`DataTable selection should render 1`] = `
role="img"
>
<svg
alt="Sort rows by this header in descending order"
aria-label="Sort rows by this header in descending order"
alt="Sort rows by this header in ascending order"
aria-label="Sort rows by this header in ascending order"
className="bx--table-sort-v2__icon"
fillRule="evenodd"
height="5"
Expand All @@ -685,7 +688,7 @@ exports[`DataTable selection should render 1`] = `
width="10"
>
<title>
Sort rows by this header in descending order
Sort rows by this header in ascending order
</title>
<path
d="M0 5L5 .002 10 5z"
Expand All @@ -706,6 +709,7 @@ exports[`DataTable selection should render 1`] = `
translateWithId={[Function]}
>
<th
aria-sort="none"
scope="col"
>
<button
Expand All @@ -719,7 +723,7 @@ exports[`DataTable selection should render 1`] = `
</span>
<Icon
className="bx--table-sort-v2__icon"
description="Sort rows by this header in descending order"
description="Sort rows by this header in ascending order"
fillRule="evenodd"
icon={
Object {
Expand Down Expand Up @@ -747,8 +751,8 @@ exports[`DataTable selection should render 1`] = `
role="img"
>
<svg
alt="Sort rows by this header in descending order"
aria-label="Sort rows by this header in descending order"
alt="Sort rows by this header in ascending order"
aria-label="Sort rows by this header in ascending order"
className="bx--table-sort-v2__icon"
fillRule="evenodd"
height="5"
Expand All @@ -757,7 +761,7 @@ exports[`DataTable selection should render 1`] = `
width="10"
>
<title>
Sort rows by this header in descending order
Sort rows by this header in ascending order
</title>
<path
d="M0 5L5 .002 10 5z"
Expand Down Expand Up @@ -1723,6 +1727,7 @@ exports[`DataTable should render 1`] = `
translateWithId={[Function]}
>
<th
aria-sort="none"
scope="col"
>
<button
Expand All @@ -1736,7 +1741,7 @@ exports[`DataTable should render 1`] = `
</span>
<Icon
className="bx--table-sort-v2__icon"
description="Sort rows by this header in descending order"
description="Sort rows by this header in ascending order"
fillRule="evenodd"
icon={
Object {
Expand Down Expand Up @@ -1764,8 +1769,8 @@ exports[`DataTable should render 1`] = `
role="img"
>
<svg
alt="Sort rows by this header in descending order"
aria-label="Sort rows by this header in descending order"
alt="Sort rows by this header in ascending order"
aria-label="Sort rows by this header in ascending order"
className="bx--table-sort-v2__icon"
fillRule="evenodd"
height="5"
Expand All @@ -1774,7 +1779,7 @@ exports[`DataTable should render 1`] = `
width="10"
>
<title>
Sort rows by this header in descending order
Sort rows by this header in ascending order
</title>
<path
d="M0 5L5 .002 10 5z"
Expand All @@ -1795,6 +1800,7 @@ exports[`DataTable should render 1`] = `
translateWithId={[Function]}
>
<th
aria-sort="none"
scope="col"
>
<button
Expand All @@ -1808,7 +1814,7 @@ exports[`DataTable should render 1`] = `
</span>
<Icon
className="bx--table-sort-v2__icon"
description="Sort rows by this header in descending order"
description="Sort rows by this header in ascending order"
fillRule="evenodd"
icon={
Object {
Expand Down Expand Up @@ -1836,8 +1842,8 @@ exports[`DataTable should render 1`] = `
role="img"
>
<svg
alt="Sort rows by this header in descending order"
aria-label="Sort rows by this header in descending order"
alt="Sort rows by this header in ascending order"
aria-label="Sort rows by this header in ascending order"
className="bx--table-sort-v2__icon"
fillRule="evenodd"
height="5"
Expand All @@ -1846,7 +1852,7 @@ exports[`DataTable should render 1`] = `
width="10"
>
<title>
Sort rows by this header in descending order
Sort rows by this header in ascending order
</title>
<path
d="M0 5L5 .002 10 5z"
Expand Down
2 changes: 1 addition & 1 deletion src/components/DataTable/state/__tests__/getDerivedStateFromProps-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('getDerivedStateFromProps', () => {
expect(initialState.rowIds).toEqual(['1', '2', '0']);
const prevState = {
sortHeaderKey: 'sortField',
sortDirection: 'DESC',
sortDirection: 'ASC',
};
const nextState = getDerivedStateFromProps(mockProps, prevState);
expect(nextState.rowIds).toEqual(['0', '1', '2']);
Expand Down
28 changes: 14 additions & 14 deletions src/components/DataTable/state/__tests__/sorting-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ describe('sorting state', () => {
mockPrevState = sortStates.NONE;
});

it('should default to DESC', () => {
it('should default to ASC', () => {
expect(
getNextSortDirection(mockHeaderA, mockHeaderA, mockPrevState)
).toBe(sortStates.DESC);
).toBe(sortStates.ASC);
});

it('should transition from DESC -> ASC -> NONE', () => {
it('should transition from ASC -> DESC -> NONE', () => {
const nextState1 = getNextSortDirection(
mockHeaderA,
mockHeaderA,
Expand All @@ -67,13 +67,13 @@ describe('sorting state', () => {
mockHeaderA,
nextState3
);
expect(nextState1).toBe(sortStates.DESC);
expect(nextState2).toBe(sortStates.ASC);
expect(nextState1).toBe(sortStates.ASC);
expect(nextState2).toBe(sortStates.DESC);
expect(nextState3).toBe(sortStates.NONE);
expect(nextState4).toBe(sortStates.DESC);
expect(nextState4).toBe(sortStates.ASC);
});

it('should reset to DESC if the header changes', () => {
it('should reset to ASC if the header changes', () => {
const nextState1 = getNextSortDirection(
mockHeaderA,
mockHeaderA,
Expand All @@ -89,9 +89,9 @@ describe('sorting state', () => {
mockHeaderB,
nextState2
);
expect(nextState1).toBe(sortStates.DESC);
expect(nextState2).toBe(sortStates.ASC);
expect(nextState3).toBe(sortStates.DESC);
expect(nextState1).toBe(sortStates.ASC);
expect(nextState2).toBe(sortStates.DESC);
expect(nextState3).toBe(sortStates.ASC);
});
});

Expand Down Expand Up @@ -130,13 +130,13 @@ describe('sorting state', () => {
};
});

it('should initialize in DESC order for the first header called', () => {
it('should initialize in ASC order for the first header called', () => {
const sortHeaderKey = 'a';
expect(
getNextSortState(mockProps, mockState, { key: sortHeaderKey })
).toEqual({
sortHeaderKey,
sortDirection: sortStates.DESC,
sortDirection: sortStates.ASC,
rowIds: ['b', 'a', 'c'],
});
});
Expand Down Expand Up @@ -168,12 +168,12 @@ describe('sorting state', () => {
);
expect(nextState1).toEqual({
sortHeaderKey,
sortDirection: sortStates.DESC,
sortDirection: sortStates.ASC,
rowIds: ['b', 'a', 'c'],
});
expect(nextState2).toEqual({
sortHeaderKey,
sortDirection: sortStates.ASC,
sortDirection: sortStates.DESC,
rowIds: ['b', 'a', 'c'],
});
expect(nextState3).toEqual({
Expand Down
Loading

0 comments on commit ee555b6

Please sign in to comment.