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

Issue 250 - Support for rendering links inside dcc.Markdown as dcc.Link #711

Merged
merged 24 commits into from
Jan 9, 2020

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Dec 3, 2019

Fixes #250

Adds additional node type for dcc.Link (vs. link) and the general mechanism to extend markdown syntax with any React Component of our choosing. Leaves the existing link syntax untouched.

Works as-is, no need to set dangerously_allow_html=True -- which I'm not sure makes sense? Need to investigate / make sure we're not doing something wrong..

Requires dangerously_allow_html=True to work correctly.

Todo

import dash
import dash_core_components as dcc

app = dash.Dash(__name__)
app.layout = dcc.Markdown("""
    # Link
    [Outside Link](https://www.google.com)
    # DCC Link!
    <dccLink href="potato" children="potato" />
    <dccLink href="with_gravy" children="with gravy" />
""")

app.run_server(debug=True)

image

@@ -58,7 +58,7 @@ module.exports = (env, argv) => {
rules: [
{
test: /\.jsx?$/,
exclude: /node_modules/,
exclude: /node_modules\/(?!react-jsx-parser\/)/,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly not transpiled to ES5 so needs to be re-transpiled.

@@ -85,6 +88,10 @@ export default class DashMarkdown extends Component {
const displayText =
dedent && textProp ? this.dedent(textProp) : textProp;

const componentTransforms = {
DccLink: props => <DccLink {...props} />,
};
Copy link
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Dec 3, 2019

Choose a reason for hiding this comment

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

Here we define the list of components we want to support are their name in markdown + props customization as needed.

@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review December 3, 2019 01:27
@chriddyp
Copy link
Member

chriddyp commented Dec 3, 2019

Very cool syntax. Should we do <DccLink or <dccLink? The latter is similar to the Python and R (cc @sudburyrob @rpkyle ) syntax but maybe less semantic in terms of XML

@@ -110,6 +117,14 @@ export default class DashMarkdown extends Component {
<Markdown
source={displayText}
escapeHtml={!dangerously_allow_html}
renderers={{
html: props => (
Copy link
Member

Choose a reason for hiding this comment

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

should we toss in a danderous HTML / XSS test for future sanity?

@chriddyp
Copy link
Member

chriddyp commented Dec 3, 2019

I like it. Nice general solution. 💃

@chriddyp
Copy link
Member

chriddyp commented Dec 3, 2019

Does this work?

<DccLink href="/lahlah">
Test
</DccLink>

Also, does this work?

<DccLink href="/lahlah">
![Alt text for an image](https://plot.ly~chris/1638.png)
</DccLink>

What about this?

<DccLink href="/lahlah" children="![Alt text for an image](https://plot.ly~chris/1638.png)">
</DccLink>

@chriddyp
Copy link
Member

chriddyp commented Dec 3, 2019

Also could we do a image test that our CSS is OK? That is, that inline dcc.Link is rendered the same way as an inline markdown link?

This is a [regular html link](/test) within a line of text.

This is a <DccLink href="/test">special dcc link</DccLink> within a line of text.

You can't tell them apart!

@chriddyp
Copy link
Member

chriddyp commented Dec 3, 2019

Sorry @sudburyrob I meant to tag @rpkyle

@Marc-Andre-Rivet
Copy link
Contributor Author

Marc-Andre-Rivet commented Dec 3, 2019

@chriddyp We're in html land now so this does not work:

<DccLink href="/lahlah">
![Alt text for an image](https://plot.ly/~chris/1638.png)
</DccLink>

But this works:

<dccLink href="lots_of_cheese">
    <img src="https://plot.ly/~chris/1638.png"/>
</dccLink>

And this works too

<DccLink href="/lahlah">
Test
</DccLink>

That said, nothing would prevent us from going all in and allowing nested markdowns inside the markdown:

<dccLink href="really_lots_of_cheese">
    <dccMarkdown source="![Alt text for an image](https://plot.ly/~chris/1638.png)" />
</dccLink>

Not advocating this, but... it works... who am I to say nested markdowns shouldn't be a thing? 🤷‍♂️

@Marc-Andre-Rivet
Copy link
Contributor Author

Should we do DccLink or dccLink? The latter is similar to the Python and R

dccLink does make more sense, and it works (does not suffer React's capitalization limitations). Updating.

Marc-Andre-Rivet and others added 5 commits December 2, 2019 20:43
@Marc-Andre-Rivet
Copy link
Contributor Author

One difference between using dccLink through the html renderer and link (e.g. [Link](/link-path)) gets wrapped in a <p>...</p> along with everything else in the line.

@Marc-Andre-Rivet Marc-Andre-Rivet added this to the Dash v1.8 milestone Dec 3, 2019
@chriddyp
Copy link
Member

chriddyp commented Dec 3, 2019

Another nice enhancement (down the line) would be to enable end users to specify the properties of their html.A like target="blank", so:

dcc.Markdown('''
Long string of text with a <a href="https://plot.ly" target="blank">absolute link</a> in it.
''')

@chriddyp
Copy link
Member

chriddyp commented Dec 3, 2019

One difference between using dccLink through the html renderer and link (e.g. Link) gets wrapped in a

...

along with everything else in the line.

Does this mean that using <dccLink/> in markdown will bring the content into a new line rather than keeping it with the text? If so, then that would reduce the ease of use of this enhancement.

@Marc-Andre-Rivet
Copy link
Contributor Author

Marc-Andre-Rivet commented Dec 3, 2019

Does this mean that using in markdown will bring the content into a new line rather than keeping it with the text?

Testing it...

dcc.Markdown(['This is an inlined <dccLink href="title_crumb" children="Title" /> with text on both sides'], dangerously_allow_html=True,),

Currently renders as:
image

Checking if something can be done about it.


With some tweaking (https://github.com/TroyAlford/react-jsx-parser renderInWrapper=False) now renders html without a wrapper, so the above is now:

image

Added a test for the inline case.

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.

For dash-docs this will work fine - we can update reusable_components.Markdown to automatically convert [title](url) to <dccLink children="title" href="url" /> or whatever, and add dangerously_allow_html=True. Given that this implementation is pretty easy (and done), let's put it in 💃(once tests pass again)

If anyone else besides dash-docs is interested in this, we still may want to explore a syntax like [title](%url) that should work without dangerously_allow_html and should allow markdown within the link text without re-wrapping it.

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.

Support for rendering links inside dcc.Markdown as dcc.Link for single page dash apps
3 participants