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

Issue 171 - Rename sorting_settings columnId -> column_id #183

Merged
merged 4 commits into from
Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ 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

Expand All @@ -422,4 +422,8 @@ Derived properties allow the component to expose complex state that can be usefu
## RC13 - Allow keyboard navigation on focused input

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

## RC14 - Rename sorting_settings columnId -> column_id

Issue: https://github.com/plotly/dash-table/issues/171
6 changes: 3 additions & 3 deletions dash_table/bundle.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions dash_table/demo.js

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dash_table/metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@
"value": {
"name": "shape",
"value": {
"columnId": {
"column_id": {
"name": "union",
"value": [
{
Expand Down Expand Up @@ -700,7 +700,7 @@
}
},
"required": false,
"description": "`sorting_settings` describes the current state\nof the sorting UI.\nThat is, if the user clicked on the sort arrow\nof a column, then this property will be updated\nwith the column ID and the direction\n(`asc` or `desc`) of the sort.\nFor multi-column sorting, this will be a list of\nsorting parameters, in the order in which they were\nclicked.\n\nNOTE - We may rename `columnId` to `column_id` in\nthe future.\nSubscribe to [https://github.com/plotly/dash-table/issues/171](https://github.com/plotly/dash-table/issues/171)\nfor details.",
"description": "`sorting_settings` describes the current state\nof the sorting UI.\nThat is, if the user clicked on the sort arrow\nof a column, then this property will be updated\nwith the column ID and the direction\n(`asc` or `desc`) of the sort.\nFor multi-column sorting, this will be a list of\nsorting parameters, in the order in which they were\nclicked.",
"defaultValue": {
"value": "[]",
"computed": false
Expand Down
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.0rc13",
"version": "3.1.0rc14",
"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.0rc13",
"version": "3.1.0rc14",
"description": "Dash table",
"main": "build/index.js",
"scripts": {
Expand Down
6 changes: 3 additions & 3 deletions src/core/sorting/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as R from 'ramda';
import { ColumnId } from 'dash-table/components/Table/props';

export interface ISortSetting {
columnId: ColumnId;
column_id: ColumnId;
direction: SortDirection;
}

Expand All @@ -25,7 +25,7 @@ export default (data: any[], settings: SortSettings, isNully: IsNullyFn = defaul
R.map(setting => {
return setting.direction === SortDirection.Descending ?
R.comparator((d1: any, d2: any) => {
const id = setting.columnId;
const id = setting.column_id;

const prop1 = d1[id];
const prop2 = d2[id];
Expand All @@ -39,7 +39,7 @@ export default (data: any[], settings: SortSettings, isNully: IsNullyFn = defaul
return prop1 > prop2;
}) :
R.comparator((d1: any, d2: any) => {
const id = setting.columnId;
const id = setting.column_id;

const prop1 = d1[id];
const prop2 = d2[id];
Expand Down
4 changes: 2 additions & 2 deletions src/core/sorting/multi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ export default (
settings = R.clone(settings);

if (setting.direction === SortDirection.None) {
const currentIndex = R.findIndex(s => s.columnId === setting.columnId, settings);
const currentIndex = R.findIndex(s => s.column_id === setting.column_id, settings);

if (currentIndex !== -1) {
settings.splice(currentIndex, 1);
}
} else {
const currentSetting = R.find(s => s.columnId === setting.columnId, settings);
const currentSetting = R.find(s => s.column_id === setting.column_id, settings);

if (currentSetting) {
currentSetting.direction = setting.direction;
Expand Down
7 changes: 1 addition & 6 deletions src/dash-table/Table.js
Original file line number Diff line number Diff line change
Expand Up @@ -584,16 +584,11 @@ export const propTypes = {
* For multi-column sorting, this will be a list of
* sorting parameters, in the order in which they were
* clicked.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing link to 171 as the issue is resolved.

*
* NOTE - We may rename `columnId` to `column_id` in
* the future.
* Subscribe to [https://github.com/plotly/dash-table/issues/171](https://github.com/plotly/dash-table/issues/171)
* for details.
*/
sorting_settings: PropTypes.arrayOf(
// .exact
PropTypes.shape({
columnId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired,
column_id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired,
direction: PropTypes.oneOf(['asc', 'desc']).isRequired
})),

Expand Down
4 changes: 2 additions & 2 deletions src/dash-table/derived/header/content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function doSort(columnId: ColumnId, sortSettings: SortSettings, sortType: Sortin
setProps({
sorting_settings: sortingStrategy(
sortSettings,
{ columnId, direction }
{ column_id: columnId, direction }
)
});
};
Expand All @@ -62,7 +62,7 @@ function editColumnName(column: IVisibleColumn, columns: VisibleColumns, columnR
}

function getSorting(columnId: ColumnId, settings: SortSettings): SortDirection {
const setting = R.find(s => s.columnId === columnId, settings);
const setting = R.find(s => s.column_id === columnId, settings);

return setting ? setting.direction : SortDirection.None;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/cypress/dash/v_be_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def updateData(pagination_settings, sorting_settings):
sorted_df = df.values
else:
sorted_df = df.sort_index(
axis=sorting_settings[0]['columnId'],
axis=sorting_settings[0]['column_id'],
ascending=(sorting_settings[0]['direction'] == 'asc')
).values

Expand Down
76 changes: 38 additions & 38 deletions tests/cypress/tests/unit/sorting_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('sort', () => {
const data = [[1], [3], [4], [2]];
const sorted = sort(
data,
[{ columnId: 0, direction: SortDirection.Descending }]
[{ column_id: 0, direction: SortDirection.Descending }]
);

expect(sorted.length).to.equal(data.length);
Expand All @@ -22,7 +22,7 @@ describe('sort', () => {

const sorted = sort(
data,
[{ columnId: 0, direction: SortDirection.Descending }]
[{ column_id: 0, direction: SortDirection.Descending }]
);

expect(sorted.length).to.equal(data.length);
Expand All @@ -40,7 +40,7 @@ describe('sort', () => {

const sorted = sort(
data,
[{ columnId: 0, direction: SortDirection.Ascending }]
[{ column_id: 0, direction: SortDirection.Ascending }]
);

expect(sorted.length).to.equal(data.length);
Expand All @@ -58,7 +58,7 @@ describe('sort', () => {

const sorted = sort(
data,
[{ columnId: 0, direction: SortDirection.Descending }]
[{ column_id: 0, direction: SortDirection.Descending }]
);

expect(sorted.length).to.equal(data.length);
Expand All @@ -76,7 +76,7 @@ describe('sort', () => {

const sorted = sort(
data,
[{ columnId: 0, direction: SortDirection.Ascending }]
[{ column_id: 0, direction: SortDirection.Ascending }]
);

expect(sorted.length).to.equal(data.length);
Expand Down Expand Up @@ -105,8 +105,8 @@ describe('sort', () => {
const sorted = sort(
data,
[
{ columnId: 0, direction: SortDirection.Descending },
{ columnId: 1, direction: SortDirection.Descending }
{ column_id: 0, direction: SortDirection.Descending },
{ column_id: 1, direction: SortDirection.Descending }
]
);

Expand Down Expand Up @@ -135,8 +135,8 @@ describe('sort', () => {
const sorted = sort(
data,
[
{ columnId: 1, direction: SortDirection.Descending },
{ columnId: 0, direction: SortDirection.Ascending }
{ column_id: 1, direction: SortDirection.Descending },
{ column_id: 0, direction: SortDirection.Ascending }
]
);

Expand All @@ -153,48 +153,48 @@ describe('sort', () => {
describe('sorting settings', () => {
describe('single column sorting', () => {
it('new descending', () => {
const settings = singleUpdateSettings([], { columnId: 0, direction: SortDirection.Descending });
const settings = singleUpdateSettings([], { column_id: 0, direction: SortDirection.Descending });

expect(settings.length).to.equal(1);
expect(settings[0].columnId).to.equal(0);
expect(settings[0].column_id).to.equal(0);
expect(settings[0].direction).to.equal(SortDirection.Descending);
});

it('update to descending', () => {
const settings = singleUpdateSettings(
[{ columnId: 0, direction: SortDirection.Ascending }],
{ columnId: 0, direction: SortDirection.Descending }
[{ column_id: 0, direction: SortDirection.Ascending }],
{ column_id: 0, direction: SortDirection.Descending }
);

expect(settings.length).to.equal(1);
expect(settings[0].columnId).to.equal(0);
expect(settings[0].column_id).to.equal(0);
expect(settings[0].direction).to.equal(SortDirection.Descending);
});

it('remove by setting to None', () => {
const settings = singleUpdateSettings(
[{ columnId: 0, direction: SortDirection.Ascending }],
{ columnId: 0, direction: SortDirection.None }
[{ column_id: 0, direction: SortDirection.Ascending }],
{ column_id: 0, direction: SortDirection.None }
);

expect(settings.length).to.equal(0);
});

it('replace with other', () => {
const settings = singleUpdateSettings(
[{ columnId: 0, direction: SortDirection.Ascending }],
{ columnId: 1, direction: SortDirection.Ascending }
[{ column_id: 0, direction: SortDirection.Ascending }],
{ column_id: 1, direction: SortDirection.Ascending }
);

expect(settings.length).to.equal(1);
expect(settings[0].columnId).to.equal(1);
expect(settings[0].column_id).to.equal(1);
expect(settings[0].direction).to.equal(SortDirection.Ascending);
});

it('replace with None', () => {
const settings = singleUpdateSettings(
[{ columnId: 0, direction: SortDirection.Ascending }],
{ columnId: 1, direction: SortDirection.None }
[{ column_id: 0, direction: SortDirection.Ascending }],
{ column_id: 1, direction: SortDirection.None }
);

expect(settings.length).to.equal(0);
Expand All @@ -203,65 +203,65 @@ describe('sorting settings', () => {

describe('multi columns sorting', () => {
it('new descending', () => {
const settings = multiUpdateSettings([], { columnId: 0, direction: SortDirection.Descending });
const settings = multiUpdateSettings([], { column_id: 0, direction: SortDirection.Descending });

expect(settings.length).to.equal(1);
expect(settings[0].columnId).to.equal(0);
expect(settings[0].column_id).to.equal(0);
expect(settings[0].direction).to.equal(SortDirection.Descending);
});

it('update to descending', () => {
const settings = multiUpdateSettings(
[{ columnId: 0, direction: SortDirection.Ascending }],
{ columnId: 0, direction: SortDirection.Descending }
[{ column_id: 0, direction: SortDirection.Ascending }],
{ column_id: 0, direction: SortDirection.Descending }
);

expect(settings.length).to.equal(1);
expect(settings[0].columnId).to.equal(0);
expect(settings[0].column_id).to.equal(0);
expect(settings[0].direction).to.equal(SortDirection.Descending);
});

it('remove by setting to None', () => {
const settings = multiUpdateSettings(
[{ columnId: 0, direction: SortDirection.Ascending }],
{ columnId: 0, direction: SortDirection.None }
[{ column_id: 0, direction: SortDirection.Ascending }],
{ column_id: 0, direction: SortDirection.None }
);

expect(settings.length).to.equal(0);
});

it('respects order', () => {
const settings = multiUpdateSettings(
[{ columnId: 0, direction: SortDirection.Ascending }],
{ columnId: 1, direction: SortDirection.Ascending }
[{ column_id: 0, direction: SortDirection.Ascending }],
{ column_id: 1, direction: SortDirection.Ascending }
);

expect(settings.length).to.equal(2);
expect(settings[0].columnId).to.equal(0);
expect(settings[1].columnId).to.equal(1);
expect(settings[0].column_id).to.equal(0);
expect(settings[1].column_id).to.equal(1);
});

it('respects order when removed and added back', () => {
let settings: SortSettings = [{ columnId: 0, direction: SortDirection.Ascending }];
let settings: SortSettings = [{ column_id: 0, direction: SortDirection.Ascending }];

settings = multiUpdateSettings(
settings,
{ columnId: 1, direction: SortDirection.Ascending }
{ column_id: 1, direction: SortDirection.Ascending }
);

settings = multiUpdateSettings(
settings,
{ columnId: 0, direction: SortDirection.None }
{ column_id: 0, direction: SortDirection.None }
);

settings = multiUpdateSettings(
settings,
{ columnId: 0, direction: SortDirection.Ascending }
{ column_id: 0, direction: SortDirection.Ascending }
);

expect(settings.length).to.equal(2);
expect(settings[0].columnId).to.equal(1);
expect(settings[1].columnId).to.equal(0);
expect(settings[0].column_id).to.equal(1);
expect(settings[1].column_id).to.equal(0);
});
});
});
8 changes: 4 additions & 4 deletions tests/visual/percy-storybook/DashTable.percy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ storiesOf('DashTable/Sorting', module)
data={sparseData}
columns={mergedColumns}
sorting={true}
sorting_settings={[{ columnId: 'a', direction: 'asc' }]}
sorting_settings={[{ column_id: 'a', direction: 'asc' }]}
style_data_conditional={style_data_conditional}
/>))
.add('"a" descending', () => (<DashTable
Expand All @@ -201,7 +201,7 @@ storiesOf('DashTable/Sorting', module)
data={sparseData}
columns={mergedColumns}
sorting={true}
sorting_settings={[{ columnId: 'a', direction: 'desc' }]}
sorting_settings={[{ column_id: 'a', direction: 'desc' }]}
style_data_conditional={style_data_conditional}
/>))
.add('"a" ascending -- empty string override', () => (<DashTable
Expand All @@ -210,7 +210,7 @@ storiesOf('DashTable/Sorting', module)
data={sparseData}
columns={mergedColumns}
sorting={true}
sorting_settings={[{ columnId: 'a', direction: 'asc' }]}
sorting_settings={[{ column_id: 'a', direction: 'asc' }]}
sorting_treat_empty_string_as_none={true}
style_data_conditional={style_data_conditional}
/>))
Expand All @@ -220,7 +220,7 @@ storiesOf('DashTable/Sorting', module)
data={sparseData}
columns={mergedColumns}
sorting={true}
sorting_settings={[{ columnId: 'a', direction: 'desc' }]}
sorting_settings={[{ column_id: 'a', direction: 'desc' }]}
sorting_treat_empty_string_as_none={true}
style_data_conditional={style_data_conditional}
/>));
Expand Down