Issue 710 - Markdown link click (#787)

Author: Marc-Andre-Rivet

.circleci/config.yml

@@ -3,7 +3,7 @@ version: 2
 jobs:
     "server-test":
         docker:
-            - image: circleci/python:3.7-node-browsers
+            - image: circleci/python:3.7.6-node-browsers
             - image: cypress/base:10
 
         steps:

CHANGELOG.md

@@ -2,6 +2,15 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [Unreleased]
+### Added
+- [#787](https://github.com/plotly/dash-table/pull/787) Add `cell_selectable` property to allow/disallow cell selection
+
+### Changed
+- [#787](https://github.com/plotly/dash-table/pull/787)
+    - Clicking on a link in a Markdown cell now requires a single click instead of two
+    - Links in Markdown cells now open a new tab (target="_blank")
+
 ## [4.7.0] - 2020-05-05
 ### Added
 - [#729](https://github.com/plotly/dash-table/pull/729) Improve conditional styling

src/dash-table/components/CellMarkdown/index.tsx

@@ -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;
@@ -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({}); });
         }
     }
 
@@ -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)}
         />);
     }
 

src/dash-table/components/EdgeFactory.tsx

@@ -186,7 +186,7 @@ export default class EdgeFactory {
     }
 
     private memoizedCreateEdges = memoizeOne((
-        active_cell: ICellCoordinates,
+        active_cell: ICellCoordinates | undefined,
         columns: Columns,
         visibleColumns: Columns,
         operations: number,

src/dash-table/components/Table/props.ts

@@ -301,6 +301,7 @@ export interface IProps {
     tooltip_conditional: ConditionalTooltip[];
 
     active_cell?: ICellCoordinates;
+    cell_selectable?: boolean;
     column_selectable?: Selection;
     columns?: Columns;
     dropdown?: StaticDropdowns;
@@ -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;
 
@@ -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;

src/dash-table/dash/DataTable.js

@@ -85,6 +85,7 @@ export const defaultProps = {
     selected_columns: [],
     selected_rows: [],
     selected_row_ids: [],
+    cell_selectable: true,
     row_selectable: false,
 
     style_table: {},
@@ -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.

src/dash-table/dash/Sanitizer.ts

@@ -16,7 +16,8 @@ import {
     ExportFormat,
     ExportHeaders,
     IFilterAction,
-    FilterLogicalOperator
+    FilterLogicalOperator,
+    SelectedCells
 } from 'dash-table/components/Table/props';
 import headerRows from 'dash-table/derived/header/headerRows';
 import resolveFlag from 'dash-table/derived/cell/resolveFlag';
@@ -33,6 +34,7 @@ const D3_DEFAULT_LOCALE: INumberLocale = {
 
 const DEFAULT_NULLY = '';
 const DEFAULT_SPECIFIER = '';
+const NULL_SELECTED_CELLS: SelectedCells = [];
 
 const data2number = (data?: any) => +data || 0;
 
@@ -99,7 +101,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 :
+            NULL_SELECTED_CELLS;
+
         return R.merge(props, {
+            active_cell,
             columns,
             data,
             export_headers: headerFormat,
@@ -108,6 +119,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
         });
     }

src/dash-table/derived/cell/wrapperStyles.ts

@@ -30,7 +30,7 @@ const getter = (
     styles: IConvertedStyle[],
     data: Data,
     offset: IViewportOffset,
-    activeCell: ICellCoordinates,
+    activeCell: ICellCoordinates | undefined,
     selectedCells: SelectedCells
 ) => {
     baseline = shallowClone(baseline);

src/dash-table/handlers/cellEvents.ts

@@ -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';
@@ -12,6 +12,7 @@ export const handleClick = (
     e: any
 ) => {
     const {
+        cell_selectable,
         selected_cells,
         active_cell,
         setProps,
@@ -29,7 +30,14 @@ export const handleClick = (
         return;
     }
 
-    e.preventDefault();
+    const column = visibleColumns[col];
+    if (column.presentation !== Presentation.Markdown) {
+        e.preventDefault();
+    }
+
+    if (!cell_selectable) {
+        return;
+    }
 
     /*
      * In some cases this will initiate browser text selection.

src/dash-table/utils/MarkdownHighlighter.tsx renamed to src/dash-table/utils/Markdown.ts

@@ -1,14 +1,14 @@
 import { Remarkable } from 'remarkable';
 import LazyLoader from 'dash-table/LazyLoader';
 
-export default class MarkdownHighlighter {
+export default class Markdown {
 
     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;
@@ -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;
     }
 }

tests/cypress/tests/standalone/markdown_test.ts

@@ -136,15 +136,6 @@ describe('markdown cells', () => {
         });
     });
 
-    describe('clicking links', () => {
-        it('correctly redirects', () => {
-            cy.visit(`http://localhost:8080?mode=${AppMode.Markdown}`);
-            // change href, since Cypress raises error when navigating away from localhost
-            DashTable.getCellById(10, 'markdown-links').within(() => cy.get('.dash-cell-value > p > a').invoke('attr', 'href', '#testlinkclick').click().click());
-            cy.url().should('include', `#testlinkclick`);
-        });
-    });
-
     describe('loading highlightjs', () => {
         it('loads highlight.js and does not attach hljs to window', () => {
             cy.visit(`http://localhost:8080?mode=${AppMode.Markdown}`);

tests/selenium/test_markdown_link.py

@@ -0,0 +1,46 @@
+import dash
+from dash_table import DataTable
+import pytest
+
+
+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 target.cell(0, "a").is_selected() == cell_selectable
+    assert len(test.driver.window_handles) == 2
+
+    # Make sure the new tab is what's expected
+    test.driver.switch_to_window(test.driver.window_handles[1])
+    assert test.driver.current_url.startswith("https://www.google.com")
+
+    # Make sure the cell is still selected iff cell_selectable, after switching tabs
+    test.driver.switch_to_window(test.driver.window_handles[0])
+    assert target.cell(0, "a").is_selected() == cell_selectable

tests/selenium/test_navigation.py

@@ -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

tests/visual/percy-storybook/Style.percy.tsx

@@ -531,7 +531,15 @@ storiesOf('DashTable/Style type condition', module)
             { row: 2, column: 1, column_id: 'b' },
             { row: 2, column: 2, column_id: 'c' }]}
         active_cell={{ row: 1, column: 1 }}
+    />))
+    .add('unselectable cells', () => (<DataTable
+        {...DEFAULT_TABLE}
+        id='unselectable-cells'
+        cell_selectable={false}
+        selected_cells={[
+            { row: 1, column: 1, column_id: 'b' },
+            { row: 1, column: 2, column_id: 'c' },
+            { row: 2, column: 1, column_id: 'b' },
+            { row: 2, column: 2, column_id: 'c' }]}
+        active_cell={{ row: 1, column: 1 }}
     />));
-
-
-