feat: Add clickable links for Table

[aalencar]

Proposed API

Set data_type to 'link' and use Markdown notation [label](url)

 ui.table(
        name='table',
        columns=[
            ui.table_column(name='link', label='Link', data_type='link')
        ],
        rows=[
            ui.table_row(name='row1', cells=['[Wave](https://wave.h2o.ai/)'])
        ]
    )

I tried to use ui.link as suggested by @mturoci but I didn't find a way to pass label to it because cells only accept strings. I wish I could do something like cells=[ui.link(label='my label', path='https://website.com')].

/** Create a table row. */
interface TableRow {
  /** An identifying name for this row. */
  name: Id
  /** The cells in this row (displayed left to right). */
  cells: S[]
}

[mturoci]

It would be good to also think about supporting links that will be able to also open external pages in new tabs (#567) and allow download as well. Also use TableCellType instead of data_type as this is a specific type of cell, not a type of data, it's a string after all.

[aalencar]

New API

export interface URLTableCellType {
  /** An identifying name for this component. */
  name: S
  /** Whether it should open a new tab. */
  new_tab?: B
}

In Action

q.page['example'] = ui.form_card(box='1 1 3 3', items=[
    ui.table(
        name='table',
        columns=[
            ui.table_column(name='link', label='Link', cell_type=ui.url_table_cell_type(name='urls', new_tab=True))
        ],
        rows=[
            ui.table_row(name='row1', cells=['[Wave](https://wave.h2o.ai/)'])
            ui.table_row(name='row2', cells=['https://wave.h2o.ai/'])
        ]
    )
])

Questions

Should it default to always open in a new tab?
Should I remove the current behavior if the user doesn't provide a URL with http(s) prefix?
From table.md:

If the URL doesn't have 'http(s)://' the link will be appended to the current URL, trying to access a page in the same domain.

I thought that maybe it could be used for routing to different apps in the same domain, so the user wouldn't have to pass the full domain. Just an idea.

[mturoci]

This is a good direction. Let's do the following changes:

• URLTableCellType -> LinkTableCellType
• new_tab -> target - for consistency with ui.link

Should it default to always open in a new tab?

No, the default should be the same as ui.link.

Should I remove the current behavior if the user doesn't provide a URL with http(s) prefix?

Can you provide a link? I was not able to find the string you quoted in table.md

I thought that maybe it could be used for routing to different apps in the same domain, so the user wouldn't have to pass the full domain.

It should work like a common web routing (relative links). I see your point, but that would prevent people who want to just append to the current URL (for some reason). Specifying a full domain for routing to a different app on the same server is fine.

[mturoci]

Thanks @aalencar! Looks good overall, need a few minor changes.

[mturoci]

Looks good, just one more nit. Will merge it after the 0.21 release.

[lo5]

I'm not clear about why this feature is required. We already support the ability for a row to be linked. If there is a need to provide row-specific actions, there's already the other PR with the contextual menu. If the need is to support links on each row that open in new windows, then the request is too specific, and the implementation/API seem hacky (regex, markdown-subset, etc.)

Is there a link to the specific feature request or issue?

[aalencar]

If there is a need to provide row-specific actions, there's already the other PR with the contextual menu

That PR implementation has every row with the same actions. We can discuss further improvements there.

Is there a link to the specific feature request or issue?

I couldn't find any issue or discussion opened by @vopani. @mturoci by any chance, do you remember what was the usecase?

If the need is to support links on each row that open in new windows, then the request is too specific, and the implementation/API seem hacky (regex, markdown-subset, etc.)

@lo5 It seems hacky indeed. What do you suggest as an alternative?

[lo5]

What do you suggest as an alternative?

Hard to say, since I'm not sure about the origin of this requirement.

One option is to have a MarkdownCellType which interprets cells as markdown. This opens up the possibility of using arbitrary formatted content (including links, e.g. [this](/foo) or [that](bar)). Implementation-wise, this can use the same markdown processor as ui.text(), with maybe some niceties like removing margins if p:only-child.

The decision on whether to open links in _blank can be based on whether the <a href= is an absolute (open in _blank) or relative URL (open in same window).

I'm not sure if the table already supports markdown formatting.

[mturoci]

The original issue is #428 (found via the branch name) and the requirement is to be able to open the links right from the table. I am fine with MarkdownCell, but make sure the spacing is correct and does not interfere with the rest of the table.

[aalencar]

The original issue is #428 (found via the branch name)

I meant where this request came from. If I see "via " I assume it comes from somewhere else, GH discussion, another GH issue, or a private conversation. #428 doesn't describe a use case. Like @lo5 said, maybe we already support the use case since it's an old feature request.

I am fine with MarkdownCell, but make sure the spacing is correct and does not interfere with the rest of the table.

Ok

[aalencar]

The decision on whether to open links in _blank can be based on whether the <a href= is an absolute (open in _blank) or relative URL (open in same window).

Markdown doesn't support setting the target to open in a new tab. One alternative would be to parse it and use XLink component to render it based on this new approach, though still using regex to extract the label and path. We would still use MarkdownTableCellType for this, just the case for links that would be treated differently.

Another option is to just use Markdown and include an example that uses plain HTML to open in new tab.

Should we also document other uses of Markdown or just point to some online reference?

[lo5]

Markdown doesn't support setting the target to open in a new tab.

Convert markdown to html, insert into DOM, use querySelectorAll() on the ref to get all a elements, inspect the paths, add an onClick handler to the ones that need to open in _blank.

[lo5]

Another option is to just use Markdown and include an example that uses plain HTML to open in new tab

I'd prefer not to encourage using HTML.

Should we also document other uses of Markdown or just point to some online reference?

Not sure what you mean by this. The markdown used in a table cell should be no different from that used elsewhere.

[aalencar]

Convert markdown to html, insert into DOM, use querySelectorAll() on the ref to get all a elements, inspect the paths, add an onClick handler to the ones that need to open in _blank.

This could also work but I wonder if there is any benefit over using regex and XLink component if the provided string is a link.

I'd prefer not to encourage using HTML.

Ok.

Not sure what you mean by this. The markdown used in a table cell should be no different from that used elsewhere.

Open in a new tab when it's an absolute path is a behavior not present in the Markdown specification, so my question was whether we need to document only this behavior and also point the user to some online reference in case they are not familiar with Markdown in general.

[lo5]

This could also work but I wonder if there is any benefit over using regex and XLink component if the provided string is a link.

The main benefit is that the technique I suggested would support arbitrary markdown, not just links.

my question was whether we need to document only this behavior

Add an attribute to MarkdownCellType that specifies whether or not external links should be opened in _blank. Default to opening all links in the same window.

[aalencar]

The main benefit is that the technique I suggested would support arbitrary markdown, not just links.

Could you please clarify this point? Don't we want to only extend the markdown link feature to support opening in a new tab? Maybe I'm missing something.

Add an attribute to MarkdownCellType that specifies whether or not external links should be opened in _blank. Default to opening all links in the same window.

I'm a little confused, should we open links in a new tab if the path is absolute and we specify that links can be opened in a new tab? Or should all links open in a new tab if that is specified on MarkdownCellType?

[lo5]

Could you please clarify this point? Don't we want to only extend the markdown link feature to support opening in a new tab? Maybe I'm missing something.

The goal is to support arbitrary links in table cells. Links can be opened in the same tab or in a new tab. Agreed?

You're suggesting using regex and XLink. I'm saying that's hacky and error-prone.

The way I see it, there's not much of a difference between supporting hyperlinks, and supporting arbitrary Markdown inside cells.

If we're only going to support hyperlinks, it raises more questions:

• What if there are 1+ hyperlinks in a cell?
• What if the hyperlinks are accompanied by text?
• What if the text requires alternate formatting?

So I don't see a good reason to support only hyperlinks. Might as well support Markdown, since Markdown already supports hyperlinks. Hence, MarkdownCellType.

That leaves one question open: do we open links in the current tab or a new tab? To which I suggested that we leave that decision to the user by adding an attribute (flag) to MarkdownCellType that specifies whether or not external links should be opened in _blank. And if they are to be opened in _blank, I suggested using the ref/querySelectorAll() technique, since that is the most robust way to implement this.

Hope this makes it clear. If not, call me.

[aalencar]

Now it's more clear, I was more confused about your suggestion to open absolute links on new tabs. Thanks a lot for clarifying. Note on implementation: I dynamically added the target attribute instead of a click handler like you suggested.

Regarding spacing, it will not wrap by default in case the text exceeds the cell. Should we wrap MarkdownCellType by default? Do you know any other case Markdown would break the cell?

[lo5]

I dynamically added the target attribute instead of a click handler like you suggested.

Good idea.

Should we wrap MarkdownCellType by default?

Yes

Do you know any other case Markdown would break the cell?

There might be, but should be enough to handle the p tag. Blockquotes, tables, headings, etc. don't make sense inside table cells anyway.

[mturoci]

LGTM, good work @aalencar!