Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #2615 - allow special characters in dash-table column IDs #2619

Merged
merged 3 commits into from
Aug 14, 2023
Merged

Fix #2615 - allow special characters in dash-table column IDs #2619

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c003bd0
Fix #2615 - allow special characters in dash-table column IDs
alexcjohnson Aug 12, 2023
bdb51ff
changelog for dash-table column IDs with special characters
alexcjohnson Aug 12, 2023
3391ee9
DRY column selectors
alexcjohnson Aug 12, 2023
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).

## Fixed

- [#2616](https://github.com/plotly/dash/pull/2616) Add mapping of tsconfig compiler option `moduleResolution`, fixes [#2618](https://github.com/plotly/dash/issues/2618)
- [#2619](https://github.com/plotly/dash/pull/2619) Fix for dash-table column IDs containing special characters
- [#2616](https://github.com/plotly/dash/pull/2616) Add mapping of tsconfig compiler option `moduleResolution`, fixes [#2618](https://github.com/plotly/dash/issues/2618)
- [#2596](https://github.com/plotly/dash/pull/2596) Fix react-dom throwing unique key prop error for markdown table, fix [#1433](https://github.com/plotly/dash/issues/1433)
- [#2589](https://github.com/plotly/dash/pull/2589) CSS for input elements not scoped to Dash application
- [#2599](https://github.com/plotly/dash/pull/2599) Fix background callback cancel inputs used in multiple callbacks and mixed cancel inputs across pages.
Expand Down
14 changes: 11 additions & 3 deletions components/dash-table/src/dash-table/components/ControlledTable/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,11 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>
active.row !== active_cell?.row)
) {
const target = this.$el.querySelector(
`td[data-dash-row="${active_cell.row}"][data-dash-column="${active_cell.column_id}"]:not(.phantom-cell)`
`td[data-dash-row="${
active_cell.row
}"][data-dash-column="${CSS.escape(
active_cell.column_id
)}"]:not(.phantom-cell)`
Copy link
Contributor

Choose a reason for hiding this comment

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

That became a bit harder to read on multiple line, I would destructure the active_cell and put the escaped column id in a variable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

dunno if it's net easier to read or not, but I DRYed this in 3391ee9

) as HTMLElement;
if (target) {
target.focus();
Expand Down Expand Up @@ -1170,10 +1174,14 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>
? table.querySelector(
`tr:nth-of-type(${
row + 1
}) th[data-dash-column="${id}"]:not(.phantom-cell)`
}) th[data-dash-column="${CSS.escape(
id
)}"]:not(.phantom-cell)`
)
: table.querySelector(
`td[data-dash-column="${id}"][data-dash-row="${row}"]:not(.phantom-cell)`
`td[data-dash-column="${CSS.escape(
id
)}"][data-dash-row="${row}"]:not(.phantom-cell)`
);

(this.refs.tooltip as TableTooltip).updateBounds(cell);
Expand Down
27 changes: 27 additions & 0 deletions components/dash-table/tests/selenium/test_column.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,30 @@ def test_colm008_top_row_by_subset(test):
for r in range(3):
if target.column(c).exists(r):
assert target.column(c).is_selected(r)


def test_colm009_newline_id(test):
app = dash.Dash(__name__)

columns = [
{"name": "aaabbb", "id": "aaa\nbbb"},
{"name": "cccddd", "id": "ccc\nddd"},
]
data = [{columns[c]["id"]: r + (3 * c) + 1 for c in [0, 1]} for r in [0, 1, 2]]
tooltip_data = [{k: str(v * 11) for k, v in row.items()} for row in data]

app.layout = DataTable(
id="table", columns=columns, data=data, tooltip_data=tooltip_data
)

test.start_server(app)

target = test.table("table")
cell = target.cell(1, 1)

target.is_ready()
cell.move_to()
# note first I tried tooltip.exists() and tooltip.get_text() like in ttip001
# but that didn't work? didn't wait for it perhaps?
test.wait_for_text_to_equal(".dash-tooltip", "55")
assert test.get_log_errors() == []
2 changes: 1 addition & 1 deletion dash/_jupyter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ def wait_for_app():
@staticmethod
def _display_in_colab(dashboard_url, port, mode, width, height):
# noinspection PyUnresolvedReferences
from google.colab import output # pylint: disable=E0401,C0415
from google.colab import output # pylint: disable=E0401,E0611,C0415

if mode == "inline":
output.serve_kernel_port_as_iframe(port, width=width, height=height)
Expand Down