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
Merged

Issue 710 - Markdown link click #787

merged 12 commits into from
Jun 1, 2020

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented May 29, 2020

Closes #710

  • Changes the behavior of markdown cells so that clicking on the cell both selects the cell and click through to the underlying element

  • Changes the default markdown link target to target=_blank to open a new tab

  • New cell_selectable prop (default: True) that turns on/off the ability to select and navigate cells in the table (also overrides value of active_cell and selected_cells during the sanitation phase)

  • Single click on markdown link opens it

  • "No selection" table mode

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

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


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.

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.

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

@Marc-Andre-Rivet Marc-Andre-Rivet changed the title [WIP] Issue 710 - Markdown link click Issue 710 - Markdown link click May 29, 2020
@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review May 29, 2020 23:32

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.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 Nice simple API addition and implementation, I like it! Just a small test addition, nonblocking if it turns out to be a pain.

Marc-André Rivet added 3 commits June 1, 2020 15:49
- make sure the cell is selected iif cell_selectable before and after switching tabs
@Marc-Andre-Rivet Marc-Andre-Rivet merged commit b510ffa into dev Jun 1, 2020
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the 710-md-click branch June 1, 2020 20:36
@ghost
Copy link

ghost commented Jun 3, 2020

Nice feature, which I was looking for, but it is not working. My app crashes with the following error:
The dash_table.DataTablecomponent (version 4.7.0) with the ID "table" received an unexpected keyword argument:cell_selectableAllowed arguments: active_cell,...

@alexcjohnson
Copy link
Collaborator

@FlorianRahe this feature has been merged but not yet released. It will be bundled with the next Dash release in a couple of weeks, or you can follow the steps in CONTRIBUTING.md to install in developer mode if you want to try it today.

@bcliang
Copy link

bcliang commented Jul 1, 2020

@Marc-Andre-Rivet How does one override the new default 'linkTarget': '_blank' to go back to the previous behavior ('_self')?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clicking on a markdown link should open the link rather than selecting the cell
3 participants