feat: Add clickable links in cell overflow modal

[emilyhuxng]

Closes #1128

Links without https are also recognized as links. I couldn't figure out how to disable the tooltip or open the link upon left click.

[codecov]

Codecov Report

Merging #1147 (cc80fa5) into main (b0865eb) will increase coverage by 0.01%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##             main    #1147      +/-   ##
==========================================
+ Coverage   44.15%   44.16%   +0.01%     
==========================================
  Files         447      447              
  Lines       33267    33276       +9     
  Branches     8356     8356              
==========================================
+ Hits        14688    14696       +8     
- Misses      18530    18531       +1     
  Partials       49       49              

Flag	Coverage Δ
unit	44.16% <88.88%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/console/src/notebook/Editor.tsx	0.00% <0.00%> (ø)
packages/console/src/monaco/MonacoUtils.ts	60.16% <100.00%> (+2.77%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[mofojed]

I noticed some inconsistency between clicking e-mail addresses vs. URIs - e-mails try to open in the same window, whereas URIs open in a separate window.
I noticed that with links: true (which should be the default according to docs? Not sure why it's not defaulting to clickable) it handles hyperlinking URIs correctly, but doesn't seem to pick up e-mails. Were there any other cases you noticed? I'm wondering if we shouldn't just use links: true until we have a way to register our own link opener (so there's no inconsistency between opening a URI vs. e-mail). Would mean having to manually copy e-mail addresses though...
Note there is an issue/PR to keep track of for registering our own custom link opener: microsoft/monaco-editor#3354

Also it is kind of jarring having a left-click open the link in grid, but then need a ctrl+click in the full editor. I also couldn't find a way to modify that behaviour in monaco, not sure if @dsmmcken has any opinions (e.g. changing Grid behaviour to require a Ctrl+Click to follow instead, or what)

[emilyhuxng]

I noticed some inconsistency between clicking e-mail addresses vs. URIs - e-mails try to open in the same window, whereas URIs open in a separate window.

This might be a browser setting? For me, it's doing the same thing as the cells. Urls open in a new tab if you click follow link, redirecting you to the tab. If you control click, it opens in a new tab in the background in the same window. Emails open the mail app.

I noticed that with links: true (which should be the default according to docs? Not sure why it's not defaulting to clickable) it handles hyperlinking URIs correctly, but doesn't seem to pick up e-mails. Were there any other cases you noticed?

Other than emails, it doesn't detect links without https like google.com. That is fine, but it would be inconsistent with the cells.

[dsmmcken]

Ctrl+click is a non-continuous selection behavior in grid, and we wouldn't want to change that.

[mofojed]

I noticed some inconsistency between clicking e-mail addresses vs. URIs - e-mails try to open in the same window, whereas URIs open in a separate window.

This might be a browser setting? For me, it's doing the same thing as the cells. Urls open in a new tab if you click follow link, redirecting you to the tab. If you control click, it opens in a new tab in the background in the same window. Emails open the mail app.

Yes, this is true. In Chrome settings in chrome://settings/handlers?search=protocol+handler, I have GMail set up to be the default email handler:

Full instructions here: https://www.indeed.com/career-advice/career-development/how-to-get-email-links-to-open-in-chrome
Though now that it's set up, I don't know how to modify that setting... annoying that it tries to open in the same window, but okay.

[dsmmcken]

Can you check styling to format the links to have $primary color. Not sure if it might be part of the monaco theme variables

[mofojed]

Instead of specifying in Editor.scss, we should add it to the MonacoTheme.module.scss. Take a look at how we create the theme in MonacoUtils.

Looking at the Monaco playground, looks like you can set a couple link options:

("textLink.foreground"); // Foreground color for links in text.
("textLink.activeForeground"); // Foreground color for active links in text.
("editorLink.activeForeground"); // Color of active links.

By adding it to the theme, we leave the door open to easily change themes.

[dsmmcken]

I am not sure that any of those three actually change it the links in the editor text.

[mofojed]

Well then. It looks like it changes the "Follow the link" colour in the popup, we should probably have that set to primary anyway no?

[dsmmcken]

Yeah. I don't know what the other one even controls.

[emilyhuxng]

I think textLink.foreground controls the color of Follow Link, textLink.activeForeground controls when you hover over Follow Link and editorLink.activeForeground is when you hold your control key down over a link. I couldn't find anything for the actual visible links.

So should all three of these be set to primary as well?

[mofojed]

@dsmmcken what styling should we use? For regular links we add in bootstrap_overrides.scss:

//links
$link-color: $gray-400;
$link-hover-color: $foreground;

But then for grid we use $primary and then don't add a hover colour.

Should we do the same for Monaco? (textLink.foreground and .detected-link as $link-color, then textLnk.activeForeground and editorLink.activeForeground as $link-hover-color)? Or use $primary

[dsmmcken]

should probably all use link-color and link-hover-color when hovered. I don't think we can set the hover start in monaco as it looks like the link is split into multiple dom elements but I could be wrong.

We should also probably change link color to primary and primary-hover.

[mofojed]

Snapshots are failing because the $link-color was changed, causing the submenu button to appear differently (shows up blue instead of the gray).
@dsmmcken I don't think we want to update $link-color to primary unless we get more of a review there about what else it affects, perhaps should be a separate change? Just have this change use $link-color without changing it.

[dsmmcken]

Yep, I see that. Agreed, revert that, and we can tackle it in a separate PR.