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

Issue 710 - Markdown link click #787

Merged
merged 12 commits into from
Jun 1, 2020
10 changes: 5 additions & 5 deletions src/dash-table/components/CellMarkdown/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import React, {
import DOM from 'core/browser/DOM';
import { memoizeOne } from 'core/memoizer';

import MarkdownHighlighter from 'dash-table/utils/MarkdownHighlighter';
import Markdown from 'dash-table/utils/Markdown';

interface IProps {
active: boolean;
Expand All @@ -18,15 +18,15 @@ export default class CellMarkdown extends PureComponent<IProps, {}> {

getMarkdown = memoizeOne((value: string, _ready: any) => ({
dangerouslySetInnerHTML: {
__html: MarkdownHighlighter.render(String(value))
__html: Markdown.render(String(value))
}
}));

constructor(props: IProps) {
super(props);

if (MarkdownHighlighter.isReady !== true) {
MarkdownHighlighter.isReady.then(() => { this.setState({}); });
if (Markdown.isReady !== true) {
Markdown.isReady.then(() => { this.setState({}); });
}
}

Expand All @@ -47,7 +47,7 @@ export default class CellMarkdown extends PureComponent<IProps, {}> {
return (<div
ref='el'
className={[className, 'cell-markdown'].join(' ')}
{...this.getMarkdown(value, MarkdownHighlighter.isReady)}
{...this.getMarkdown(value, Markdown.isReady)}
/>);
}

Expand Down
2 changes: 1 addition & 1 deletion src/dash-table/components/EdgeFactory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export default class EdgeFactory {
}

private memoizedCreateEdges = memoizeOne((
active_cell: ICellCoordinates,
active_cell: ICellCoordinates | undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typing information was inaccurate.

columns: Columns,
visibleColumns: Columns,
operations: number,
Expand Down
18 changes: 10 additions & 8 deletions src/dash-table/components/Table/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ export interface IProps {
tooltip_conditional: ConditionalTooltip[];

active_cell?: ICellCoordinates;
cell_selectable?: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New optional prop. If false, user can't select cells and active_cell and selected_cells will always be treated as null/empty.

column_selectable?: Selection;
columns?: Columns;
dropdown?: StaticDropdowns;
Expand Down Expand Up @@ -351,35 +352,35 @@ export interface IProps {
}

interface IDefaultProps {
active_cell: ICellCoordinates;
cell_selectable: boolean;
column_selectable: Selection;
css: IStylesheetRule[];
dropdown: StaticDropdowns;
dropdown_conditional: ConditionalDropdowns;
dropdown_data: DataDropdowns;
css: IStylesheetRule[];
editable: boolean;
end_cell: ICellCoordinates;
export_columns: ExportColumns;
export_format: ExportFormat;
export_headers: ExportHeaders;
fill_width: boolean;
filter_query: string;
filter_action: TableAction;
include_headers_on_copy_paste: boolean;
merge_duplicate_headers: boolean;
fixed_columns: Fixed;
fixed_rows: Fixed;
include_headers_on_copy_paste: boolean;
merge_duplicate_headers: boolean;
row_deletable: boolean;
row_selectable: Selection;
selected_cells: SelectedCells;
selected_columns: string[];
start_cell: ICellCoordinates;
end_cell: ICellCoordinates;
selected_rows: Indices;
selected_row_ids: RowId[];
selected_rows: Indices;
sort_action: TableAction;
sort_by: SortBy;
sort_mode: SortMode;
sort_as_null: SortAsNull;
start_cell: ICellCoordinates;
style_as_list_view: boolean;
tooltip_data: DataTooltips;

Expand Down Expand Up @@ -475,8 +476,9 @@ export type HeaderFactoryProps = ControlledTableProps & {
};

export interface ICellFactoryProps {
active_cell: ICellCoordinates;
active_cell?: ICellCoordinates;
applyFocus?: boolean;
cell_selectable: boolean;
dropdown: StaticDropdowns;
dropdown_conditional: ConditionalDropdowns;
dropdown_data: DataDropdowns;
Expand Down
7 changes: 7 additions & 0 deletions src/dash-table/dash/DataTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export const defaultProps = {
selected_columns: [],
selected_rows: [],
selected_row_ids: [],
cell_selectable: true,
row_selectable: false,

style_table: {},
Expand Down Expand Up @@ -622,6 +623,12 @@ export const propTypes = {
*/
row_deletable: PropTypes.bool,

/**
* If True (default), then it is possible to click and navigate
* table cells.
*/
cell_selectable: PropTypes.bool,

/**
* If `single`, then the user can select a single row
* via a radio button that will appear next to each row.
Expand Down
11 changes: 11 additions & 0 deletions src/dash-table/dash/Sanitizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import headerRows from 'dash-table/derived/header/headerRows';
import resolveFlag from 'dash-table/derived/cell/resolveFlag';
import dataLoading from 'dash-table/derived/table/data_loading';
import { defaultProps } from './DataTable';

const D3_DEFAULT_LOCALE: INumberLocale = {
symbol: ['$', ''],
Expand Down Expand Up @@ -99,7 +100,16 @@ export default class Sanitizer {
headerFormat = ExportHeaders.Ids;
}

const active_cell = props.cell_selectable ?
props.active_cell :
undefined;

const selected_cells = props.cell_selectable ?
props.selected_cells :
defaultProps.selected_cells
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sure active_cell and selected_cells have default values if not selectable.


return R.merge(props, {
active_cell,
columns,
data,
export_headers: headerFormat,
Expand All @@ -108,6 +118,7 @@ export default class Sanitizer {
fixed_rows: getFixedRows(props.fixed_rows, columns, props.filter_action),
loading_state: dataLoading(props.loading_state),
locale_format,
selected_cells,
visibleColumns
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/dash-table/derived/cell/wrapperStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const getter = (
styles: IConvertedStyle[],
data: Data,
offset: IViewportOffset,
activeCell: ICellCoordinates,
activeCell: ICellCoordinates | undefined,
selectedCells: SelectedCells
) => {
baseline = shallowClone(baseline);
Expand Down
12 changes: 10 additions & 2 deletions src/dash-table/handlers/cellEvents.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { min, max, set, lensPath } from 'ramda';
import { ICellFactoryProps } from 'dash-table/components/Table/props';
import { ICellFactoryProps, Presentation } from 'dash-table/components/Table/props';
import isActive from 'dash-table/derived/cell/isActive';
import isSelected from 'dash-table/derived/cell/isSelected';
import { makeCell, makeSelection } from 'dash-table/derived/cell/cellProps';
Expand All @@ -12,6 +12,7 @@ export const handleClick = (
e: any
) => {
const {
cell_selectable,
selected_cells,
active_cell,
setProps,
Expand All @@ -29,7 +30,14 @@ export const handleClick = (
return;
}

e.preventDefault();
const column = visibleColumns[col];
if (column.presentation !== Presentation.Markdown) {
e.preventDefault();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allow click-through to the most nested target element for Markdown cells.


if (!cell_selectable) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opt out of selection processing if the table doesn't have selectable cells.


/*
* In some cases this will initiate browser text selection.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { Remarkable } from 'remarkable';
import LazyLoader from 'dash-table/LazyLoader';

export default class MarkdownHighlighter {
export default class Markdown {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed as this helper doesn't only cover highlights now.


static isReady: Promise<boolean> | true = new Promise<boolean>(resolve => {
MarkdownHighlighter.hljsResolve = resolve;
Markdown.hljsResolve = resolve;
});

static render = (value: string) => {
return MarkdownHighlighter.md.render(value);
return Markdown.md.render(value);
}

private static hljsResolve: () => any;
Expand All @@ -17,26 +17,27 @@ export default class MarkdownHighlighter {

private static readonly md: Remarkable = new Remarkable({
highlight: (str: string, lang: string) => {
if (MarkdownHighlighter.hljs) {
if (lang && MarkdownHighlighter.hljs.getLanguage(lang)) {
if (Markdown.hljs) {
if (lang && Markdown.hljs.getLanguage(lang)) {
try {
return MarkdownHighlighter.hljs.highlight(lang, str).value;
return Markdown.hljs.highlight(lang, str).value;
} catch (err) { }
}

try {
return MarkdownHighlighter.hljs.highlightAuto(str).value;
return Markdown.hljs.highlightAuto(str).value;
} catch (err) { }
} else {
MarkdownHighlighter.loadhljs();
Markdown.loadhljs();
}
return '';
}
},
linkTarget:'_blank'
});

private static async loadhljs() {
MarkdownHighlighter.hljs = await LazyLoader.hljs;
MarkdownHighlighter.hljsResolve();
MarkdownHighlighter.isReady = true;
Markdown.hljs = await LazyLoader.hljs;
Markdown.hljsResolve();
Markdown.isReady = true;
}
}
49 changes: 49 additions & 0 deletions tests/selenium/test_markdown_link.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import dash
from dash_table import DataTable
import pandas as pd
import pytest

url = "https://github.com/plotly/datasets/raw/master/" "26k-consumer-complaints.csv"
rawDf = pd.read_csv(url)
rawDf["Complaint ID"] = rawDf["Complaint ID"].map(lambda x: "**" + str(x) + "**")
rawDf["Product"] = rawDf["Product"].map(lambda x: "[" + str(x) + "](plot.ly)")
rawDf["Issue"] = rawDf["Issue"].map(
lambda x: "![" + str(x) + "](https://dash.plot.ly/assets/images/logo.png)"
)
rawDf["State"] = rawDf["State"].map(lambda x: '```python\n"{}"\n```'.format(x))

df = rawDf.to_dict("rows")


def get_app(cell_selectable):
md = "[Click me](https://www.google.com)"

data = [
dict(a=md, b=md),
dict(a=md, b=md),
]

app = dash.Dash(__name__)

app.layout = DataTable(
id="table",
columns=[
dict(name="a", id="a", type="text", presentation="markdown"),
dict(name="b", id="b", type="text", presentation="markdown"),
],
data=data,
cell_selectable=cell_selectable,
)

return app


@pytest.mark.parametrize("cell_selectable", [True, False])
def test_tmdl001_copy_markdown_to_text(test, cell_selectable):
test.start_server(get_app(cell_selectable))

target = test.table("table")

assert len(test.driver.window_handles) == 1
target.cell(0, "a").get().find_element_by_css_selector("a").click()
assert len(test.driver.window_handles) == 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only thing I might add here is (1) verify that the new window actually went to google, and (2) switch back to the first window and verify that the cell is selected iff cell_selectable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

18 changes: 18 additions & 0 deletions tests/selenium/test_navigation.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,21 @@ def test_navg004_keyboard_between_md_and_standard_cells(test, props):
test.send_keys(Keys.ARROW_RIGHT)
test.send_keys(Keys.ARROW_DOWN)
assert target.cell(i, i).is_focused()


@pytest.mark.parametrize("cell_selectable", [True, False])
def test_navg005_unselectable_cells(test, cell_selectable):
app = dash.Dash(__name__)
app.layout = DataTable(
id="table",
columns=[dict(id="a", name="a"), dict(id="b", name="b")],
data=[dict(a=0, b=0), dict(a=1, b=2)],
cell_selectable=cell_selectable,
)

test.start_server(app)

target = test.table("table")
target.cell(0, "a").click()

assert target.cell(0, "a").is_selected() == cell_selectable