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

Issue 384 - Handling of None data and columns props #731

Merged
merged 7 commits into from
Apr 20, 2020
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
### Fixed
- [#722](https://github.com/plotly/dash-table/pull/722) Fix a bug where row height is misaligned when using fixed_columns and/or fixed_rows
- [#728](https://github.com/plotly/dash-table/pull/728) Fix copy/paste on readonly cells
- [#731](https://github.com/plotly/dash-table/pull/731) Fix a bug where `data=None` and `columns=None` caused the table to throw an error

## [4.6.2] - 2020-04-01
### Changed
Expand Down
5 changes: 5 additions & 0 deletions pytest.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[pytest]
testpaths = tests/
addopts = -rsxX -vv
log_format = %(asctime)s | %(levelname)s | %(name)s:%(lineno)d | %(message)s
log_cli_level = ERROR
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as for DCC - accelerates -k test runs
plotly/dash-core-components#740

4 changes: 2 additions & 2 deletions src/dash-table/components/Table/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,12 +341,10 @@ export interface IProps {
interface IDefaultProps {
active_cell: ICellCoordinates;
column_selectable: Selection;
columns: Columns;
dropdown: StaticDropdowns;
dropdown_conditional: ConditionalDropdowns;
dropdown_data: DataDropdowns;
css: IStylesheetRule[];
data: Data;
editable: boolean;
export_columns: ExportColumns;
export_format: ExportFormat;
Expand Down Expand Up @@ -410,6 +408,8 @@ export type PropsWithDefaults = IProps & IDefaultProps;

export type SanitizedProps = Omit<Omit<
Merge<PropsWithDefaults, {
columns: Columns;
data: Data;
fixed_columns: number;
fixed_rows: number;
loading_state: boolean;
Expand Down
2 changes: 0 additions & 2 deletions src/dash-table/dash/DataTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ export const defaultProps = {
tooltip_duration: 2000,

column_selectable: false,
columns: [],
data: [],
editable: false,
export_columns: 'visible',
export_format: 'none',
Expand Down
8 changes: 6 additions & 2 deletions src/dash-table/dash/Sanitizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ const getVisibleColumns = (
export default class Sanitizer {
sanitize(props: PropsWithDefaults): SanitizedProps {
const locale_format = this.applyDefaultToLocale(props.locale_format);
const columns = this.applyDefaultsToColumns(locale_format, props.sort_as_null, props.columns, props.editable);
const columns = props.columns ?
this.applyDefaultsToColumns(locale_format, props.sort_as_null, props.columns, props.editable) :
[];
const data = props.data ?? [];
const visibleColumns = this.getVisibleColumns(columns, props.hidden_columns);

let headerFormat = props.export_headers;
Expand All @@ -90,9 +93,10 @@ export default class Sanitizer {

return R.merge(props, {
columns,
data,
export_headers: headerFormat,
fixed_columns: getFixedColumns(props.fixed_columns, props.row_deletable, props.row_selectable),
fixed_rows: getFixedRows(props.fixed_rows, props.columns, props.filter_action),
fixed_rows: getFixedRows(props.fixed_rows, columns, props.filter_action),
loading_state: dataLoading(props.loading_state),
locale_format,
visibleColumns
Expand Down
2 changes: 1 addition & 1 deletion src/dash-table/dash/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function validColumns(props: any) {
columns
} = props;

return !R.any((column: any) =>
return R.isNil(columns) || !R.any((column: any) =>
column.format && (
(
column.format.symbol &&
Expand Down
46 changes: 46 additions & 0 deletions tests/selenium/test_empty.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import dash
from dash.dependencies import Input, Output
from dash.exceptions import PreventUpdate

from dash_table import DataTable
from dash_html_components import Button, Div


def get_app():
app = dash.Dash(__name__)

columns = [{"name": i, "id": i} for i in ["a", "b"]]
data = [dict(a=1, b=2), dict(a=11, b=22)]

app.layout = Div(
[
Button(id="clear-table", children=["Clear table"]),
DataTable(id="table", columns=columns, data=data),
]
)

@app.callback(
[Output("table", "data"), Output("table", "columns")],
[Input("clear-table", "n_clicks")],
)
def clear_table(n_clicks):
if n_clicks is None:
raise PreventUpdate

nonlocal columns, data

return (data, columns) if n_clicks % 2 == 0 else (None, None)

return app


def test_empt001_clear_(test):
test.start_server(get_app())

target = test.table("table")

assert target.is_ready()
assert len(test.driver.find_elements_by_css_selector("tr")) == 3
test.driver.find_element_by_css_selector("#clear-table").click()
assert target.is_ready()
assert len(test.driver.find_elements_by_css_selector("tr")) == 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once the callback returns, the table should (1) load successfully and get to the ready state, (2) should contain no rows.

TDD'ed locally, can undo redo the fix in the PR if we'd rather have the demonstration.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm fine with not seeing the test fail on CI - I assume we're all running our own tests with the fix disabled. But it would be nice to have this test check for the proper number of <tr> elements before the clear-table.

Also, given that you have the is_ready() method this may not be helpful, but in wildcards I added a wait_for_no_elements method