Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 72
Issue 710 - Markdown link click#787
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
86edbb69257fc52abf1c1807ff692539425b2747a2bfbd5db026c291a5506cb617fa0ef20442f58dc859File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -301,6 +301,7 @@ export interface IProps { | ||
| tooltip_conditional: ConditionalTooltip[]; | ||
| active_cell?: ICellCoordinates; | ||
| cell_selectable?: boolean; | ||
ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New optional prop. If false, user can't select cells and | ||
| 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; | ||
| 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'; | ||
| @@ -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(); | ||
| } | ||
ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
| } | ||
ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
| 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 { | ||
ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
| @@ -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; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. | ||
| # 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 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typing information was inaccurate.