Skip to content
This repository was archived by the owner on Jun 4, 2024. It is now read-only.

Sorting sorts ascending first and displays correct icon #164

Merged
merged 12 commits into from
Oct 24, 2018
Merged
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -399,4 +399,8 @@ Derived properties allow the component to expose complex state that can be usefu

## RC8 - Improve props typing

Issue: https://github.com/plotly/dash-table/issues/143
Issue: https://github.com/plotly/dash-table/issues/143

## RC9 - Sort ascending on first click
- Sorts ascending when first clicked, [#118](https://github.com/plotly/dash-table/issues/118)
- Flips icons displayed so that they are pointing up on ascending and down on descending.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include a link to the issue here since we have it.

4 changes: 2 additions & 2 deletions dash_table/bundle.js

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dash_table/demo.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dash_table/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "dash-table",
"version": "3.1.0rc8",
"version": "3.1.0rc9",
"description": "Dash table",
"main": "build/index.js",
"scripts": {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "dash-table",
"version": "3.1.0rc8",
"version": "3.1.0rc9",
"description": "Dash table",
"main": "build/index.js",
"scripts": {
Expand Down
16 changes: 8 additions & 8 deletions src/dash-table/Table.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ import Logger from 'core/Logger';

import genRandomId from './utils/generate';

function isFrontEnd(value) {
return ['fe', true, false].indexOf(value) !== -1;
}

function isBackEnd(value) {
return ['be', false].indexOf(value) !== -1;
}

export default class Table extends Component {
constructor(props) {
super(props);
Expand All @@ -23,14 +31,6 @@ export default class Table extends Component {
pagination_mode
} = this.props;

function isFrontEnd(value: any) {
return ['fe', true, false].indexOf(value) !== -1;
}

function isBackEnd(value: any) {
return ['be', false].indexOf(value) !== -1;
}

const isValid = isFrontEnd(pagination_mode) ||
(isBackEnd(filtering) && isBackEnd(sorting));

Expand Down
12 changes: 7 additions & 5 deletions src/dash-table/derived/header/content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@ function doSort(columnId: ColumnId, sortSettings: SortSettings, sortType: Sortin
let direction: SortDirection;
switch (getSorting(columnId, sortSettings)) {
case SortDirection.Descending:
direction = SortDirection.Ascending;
direction = SortDirection.None;
break;
case SortDirection.Ascending:
direction = SortDirection.None;
direction = SortDirection.Descending;
break;
case SortDirection.None:
direction = SortDirection.Ascending;
break;
default:
direction = SortDirection.Descending;
direction = SortDirection.Ascending;
break;
}

Expand Down Expand Up @@ -68,9 +70,9 @@ function getSorting(columnId: ColumnId, settings: SortSettings): SortDirection {
function getSortingIcon(columnId: ColumnId, sortSettings: SortSettings) {
switch (getSorting(columnId, sortSettings)) {
case SortDirection.Descending:
return '↑';
case SortDirection.Ascending:
return '↓';
case SortDirection.Ascending:
return '↑';
case SortDirection.None:
default:
return '↕';
Expand Down
6 changes: 3 additions & 3 deletions tests/cypress/tests/server/copy_paste_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('copy paste', () => {
cy.get('tr th.column-2 .sort').last().click();

DashTable.getCell(0, 0).click();
DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.value', '103'));
DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.value', '11'));

DOM.focused.type(`${Key.Meta}c`);

Expand All @@ -113,7 +113,7 @@ describe('copy paste', () => {
.within(() => cy.get('.dash-cell-value').should('have.html', 'MODIFIED'));
DashTable
.getCell(1, 0)
.within(() => cy.get('.dash-cell-value').should('have.value', '103'));
.within(() => cy.get('.dash-cell-value').should('have.value', '11'));

DashTable.getCell(1, 1).click();
DOM.focused.type(`${Key.Meta}c`);
Expand All @@ -126,7 +126,7 @@ describe('copy paste', () => {
.within(() => cy.get('.dash-cell-value').should('have.value', 'MODIFIED'));
DashTable
.getCell(1, 0)
.within(() => cy.get('.dash-cell-value').should('have.html', '103'));
.within(() => cy.get('.dash-cell-value').should('have.html', '11'));
});
});
});
2 changes: 1 addition & 1 deletion tests/cypress/tests/server/delete_row_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('delete', () => {
});

it('can delete row when sorted', () => {
cy.get('tr th.column-0 .sort').last().click({ force: true });
cy.get('tr th.column-0 .sort').last().click({ force: true }).click({ force: true });
DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.html', '28155'));
DashTable.getDelete(0).click();
DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.html', '28154'));
Expand Down
2 changes: 1 addition & 1 deletion tests/cypress/tests/standalone/delete_row_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('delete', () => {
});

it('can delete row when sorted', () => {
cy.get('tr th.column-0 .sort').last().click({ force: true });
cy.get('tr th.column-0 .sort').last().click({ force: true }).click({ force: true });
DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.html', '4999'));
DashTable.getDelete(0).click();
DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.html', '4998'));
Expand Down
2 changes: 1 addition & 1 deletion tests/cypress/tests/standalone/select_row_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('select row', () => {
it('select, sort, new row is not selected', () => {
DashTable.getSelect(0).within(() => cy.get('input').click());
DashTable.getSelect(0).within(() => cy.get('input').should('be.checked'));
cy.get('tr th.column-0 .sort').last().click({ force: true });
cy.get('tr th.column-0 .sort').last().click({ force: true }).click({ force: true });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double-click to sort twice so the values actually change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this is the only case where it matters throughout the existing tests as here the derived_viewport_data was still in the same order as the original data.

DashTable.getSelect(0).within(() => cy.get('input').should('not.be.checked'));
});
});
Expand Down
8 changes: 4 additions & 4 deletions tests/cypress/tests/standalone/sorting_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ describe('sort', () => {
DashTable.getCell(2, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Tropical Beaches'));
DashTable.getCell(3, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
cy.get('tr th.column-6 .sort').last().click();
DashTable.getCell(0, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Wet'));
DashTable.getCell(1, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Wet'));
DashTable.getCell(2, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Wet'));
DashTable.getCell(3, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Wet'));
DashTable.getCell(0, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
DashTable.getCell(1, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
DashTable.getCell(2, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
DashTable.getCell(3, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
});
});