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

Issue 524 - Readonly dropdown column content #530

Merged
merged 10 commits into from
Aug 5, 2019
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
9 changes: 6 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,19 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- Additionally clearing the column will reset the filter for the affected column(s)

[#318](https://github.com/plotly/dash-table/issues/318)
- Headers are included when copying from the table to different
tabs and elsewhere. They are ignored when copying from the table onto itself and
between two tables within the same tab.
- Headers are included when copying from the table to different
tabs and elsewhere. They are ignored when copying from the table onto itself and
between two tables within the same tab.

### Changed
[#497](https://github.com/plotly/dash-table/pull/497)
- Like for clearing above, deleting through the `x` action will also
reset the filter for the affected column(s)

### Fixed
[#524](https://github.com/plotly/dash-table/issues/524)
- Fixed readonly dropdown cells content (display label, not value)

[#259](https://github.com/plotly/dash-table/issues/259)
- Fixed columns `sticky` on Safari

Expand Down
9 changes: 8 additions & 1 deletion demo/AppMode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,14 @@ function getBaseTableProps(mock: IDataMock) {
bbb: {
clearable: true,
options: ['Humid', 'Wet', 'Snowy', 'Tropical Beaches'].map(i => ({
label: i,
label: `label: ${i}`,
value: i
}))
},
'bbb-readonly': {
clearable: true,
options: ['Humid', 'Wet', 'Snowy', 'Tropical Beaches'].map(i => ({
label: `label: ${i}`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Differentiate between the label and value to catch these differences in the tests.

value: i
}))
}
Expand Down
2 changes: 2 additions & 0 deletions demo/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export const generateMockData = (rows: number) => unpackIntoColumnsAndData([
id: 'bbb-readonly',
name: ['', 'Weather', 'Climate-RO'],
type: ColumnType.Text,
presentation: 'dropdown',
editable: false,
data: gendata(
i => ['Humid', 'Wet', 'Snowy', 'Tropical Beaches'][i % 4],
Expand All @@ -86,6 +87,7 @@ export const generateMockData = (rows: number) => unpackIntoColumnsAndData([
id: 'aaa-readonly',
name: ['', 'Weather', 'Temperature-RO'],
type: ColumnType.Numeric,
presentation: 'dropdown',
editable: false,
data: gendata(i => i + 1, rows)
}
Expand Down
20 changes: 17 additions & 3 deletions src/dash-table/derived/cell/contents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const mapRow = R.addIndex<IColumn, JSX.Element>(R.map);

enum CellType {
Dropdown,
DropdownLabel,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make it easier to distinguish between the general label case and the dropdown one. Processing is different.

Input,
Label
}
Expand All @@ -40,7 +41,7 @@ function getCellType(
case Presentation.Input:
return (!active || !editable) ? CellType.Label : CellType.Input;
case Presentation.Dropdown:
return (!dropdown || !editable) ? CellType.Label : CellType.Dropdown;
return (!dropdown || !editable) ? CellType.DropdownLabel : CellType.Dropdown;
default:
return (!active || !editable) ? CellType.Label : CellType.Input;
}
Expand Down Expand Up @@ -124,7 +125,9 @@ class Contents {
'dash-cell-value'
].join(' ');

switch (getCellType(active, column.editable, dropdown && dropdown.options, column.presentation)) {
const cellType = getCellType(active, column.editable, dropdown && dropdown.options, column.presentation);

switch (cellType) {
case CellType.Dropdown:
return (<CellDropdown
key={`column-${columnIndex}`}
Expand All @@ -148,15 +151,26 @@ class Contents {
type={column.type}
value={datum[column.id]}
/>);
case CellType.DropdownLabel:
case CellType.Label:
default:
const resolvedValue = cellType === CellType.DropdownLabel ?
this.resolveDropdownLabel(dropdown, datum[column.id]) :
formatters[columnIndex](datum[column.id]);

return (<CellLabel
className={className}
key={`column-${columnIndex}`}
onClick={this.handlers(Handler.Click, rowIndex, columnIndex)}
onDoubleClick={this.handlers(Handler.DoubleClick, rowIndex, columnIndex)}
value={formatters[columnIndex](datum[column.id])}
value={resolvedValue}
/>);
}
}

private resolveDropdownLabel(dropdown: IDropdown | undefined, value: any) {
const dropdownValue = dropdown && dropdown.options && dropdown.options.find(option => option.value === value);

return dropdownValue ? dropdownValue.label : value;
}
}
2 changes: 1 addition & 1 deletion tests/cypress/tests/standalone/filtering_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('filter', () => {
DashTable.getFilterById('ccc').click();

DashTable.getFilterById('bbb').within(() => cy.get('input').should('have.value', 'Tr'));
DashTable.getCellById(0, 'bbb-readonly').within(() => cy.get('.dash-cell-value').should('have.html', 'Tropical Beaches'));
DashTable.getCellById(0, 'bbb-readonly').within(() => cy.get('.dash-cell-value').should('have.html', 'label: Tropical Beaches'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update tests involving dropdowns so as to expect the label: prefix

});

it('filters `Numeric` columns with `equal` without operator', () => {
Expand Down
26 changes: 18 additions & 8 deletions tests/cypress/tests/standalone/readonly_sorting_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,25 @@ Object.values([AppMode.ReadOnly]).forEach(mode => {
});

it('can sort', () => {
DashTable.getCell(0, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'Wet'));
DashTable.getCell(1, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'Snowy'));
DashTable.getCell(2, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'Tropical Beaches'));
DashTable.getCell(3, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'Humid'));
DashTable.getCell(0, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Wet'));
DashTable.getCell(1, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Snowy'));
DashTable.getCell(2, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Tropical Beaches'));
DashTable.getCell(3, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));

DashTable.getCell(0, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Wet'));
DashTable.getCell(1, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Snowy'));
DashTable.getCell(2, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Tropical Beaches'));
DashTable.getCell(3, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also check the readonly column

cy.get('tr th.column-6 .sort').last().click();
DashTable.getCell(0, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'Humid'));
DashTable.getCell(1, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'Humid'));
DashTable.getCell(2, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'Humid'));
DashTable.getCell(3, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'Humid'));
DashTable.getCell(0, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(1, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(2, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(3, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));

DashTable.getCell(0, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also check the readonly column

DashTable.getCell(1, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(2, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(3, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
});
});
});
26 changes: 18 additions & 8 deletions tests/cypress/tests/standalone/readwrite_sorting_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,25 @@ Object.values(ReadWriteModes).forEach(mode => {
});

it('can sort', () => {
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', 'Snowy'));
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'));
DashTable.getCell(0, 6).within(() => cy.get('.Select-value-label').should('have.html', 'label: Wet'));
DashTable.getCell(1, 6).within(() => cy.get('.Select-value-label').should('have.html', 'label: Snowy'));
DashTable.getCell(2, 6).within(() => cy.get('.Select-value-label').should('have.html', 'label: Tropical Beaches'));
DashTable.getCell(3, 6).within(() => cy.get('.Select-value-label').should('have.html', 'label: Humid'));

DashTable.getCell(0, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Wet'));
DashTable.getCell(1, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Snowy'));
DashTable.getCell(2, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Tropical Beaches'));
DashTable.getCell(3, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also check the readonly column

cy.get('tr th.column-6 .sort').last().click();
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'));
DashTable.getCell(0, 6).within(() => cy.get('.Select-value-label').should('have.html', 'label: Humid'));
DashTable.getCell(1, 6).within(() => cy.get('.Select-value-label').should('have.html', 'label: Humid'));
DashTable.getCell(2, 6).within(() => cy.get('.Select-value-label').should('have.html', 'label: Humid'));
DashTable.getCell(3, 6).within(() => cy.get('.Select-value-label').should('have.html', 'label: Humid'));

DashTable.getCell(0, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(1, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(2, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(3, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also check the readonly column

});
});
});
42 changes: 42 additions & 0 deletions tests/visual/percy-storybook/Dropdown.percy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,48 @@ const columns2 = R.map(
);

storiesOf('DashTable/Dropdown', module)
.add('readonly dropdown shows label', () => <DataTable
setProps={setProps}
id='table'
data={data}
columns={columns}
editable={false}
dropdown={{
climate: {
options: R.map(
i => ({ label: `label: ${i}`, value: i }),
['Sunny', 'Snowy', 'Rainy']
)
},
city: {
options: R.map(
i => ({ label: `label: ${i}`, value: i }),
['NYC', 'Montreal', 'Miami']
)
}
}}
/>)
.add('editable dropdown shows label', () => <DataTable
setProps={setProps}
id='table'
data={data}
columns={columns}
editable={true}
dropdown={{
climate: {
options: R.map(
i => ({ label: `label: ${i}`, value: i }),
['Sunny', 'Snowy', 'Rainy']
)
},
city: {
options: R.map(
i => ({ label: `label: ${i}`, value: i }),
['NYC', 'Montreal', 'Miami']
)
}
}}
/>)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

visual tests for readonly and editable dropdowns

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used in conjonction with the updated standalone tests above

.add('dropdown by column', () => (<DataTable
setProps={setProps}
id='table'
Expand Down