text
text copied to clipboard
Link preview widgets
Implement link preview widgets for paragraphs that only contain a link.
Whoohoo, super nice :heart_eyes: Works like a charm.
I wonder though, whether there should be an option to remove the preview widget. Imagine e.g. a list of links. Legibility would decrease with the huge preview widgets in between each item in my eyes.
Another frontend comment: I think it would look better if we displayed either the link or the widget. Now the link is always duplicated. Once the typed text and a second time at the bottom of the widget.
Even though, thinking about it, no idea how the link could be updated if only the widget is displayed.
I get 404 responses for open-graph link previews (/core/references/preview/3097fca9b1ec8942c4305e550ef1b50a)
Do you have memcache enabled?
The RenderReferenceEvent is not emitted by Text so the reference frontend stuff is not loaded with Text so we get Widget for rich object type integration_github not registered for every provider.
Ah good point, will add that.
Another frontend comment: I think it would look better if we displayed either the link or the widget. Now the link is always duplicated. Once the typed text and a second time at the bottom of the widget.
Even though, thinking about it, no idea how the link could be updated if only the widget is displayed.
Yes, updating was the main reason to always show it. About the option to hide it, I'd agree that it would be nice to have, but we cannot really persist it in markdown, so it would need a per user setting that we store or something like this.
Design feedback moved to review :)
I get 404 responses for open-graph link previews (/core/references/preview/3097fca9b1ec8942c4305e550ef1b50a)
Do you have memcache enabled?
I realized right after writing my comment that i had commented the caching stuff in the ReferenceManager. All good now. Sorry for the noise.
The RenderReferenceEvent is not emitted by Text so the reference frontend stuff is not loaded with Text so we get Widget for rich object type integration_github not registered for every provider.
Ah good point, will add that.
I tried dispatching the RenderReferenceEvent event in lib/Listeners/LoadViewerListener.php
right after loading the 'text-viewer' script and it works fine.
@mejo- agree, we discussed before there should ideally be a "close x" on the top right to hide the preview, but decided it's something for a future iteration.
And we also talked about showing both vs only showing the preview, and came to the same conclusion as you with the editing issue.
In a future version, we could make it so that only the preview is shown, and it has a 3-dot menu for "Edit link" and "Hide preview"
About the option to hide it, I'd agree that it would be nice to have, but we cannot really persist it in markdown, so it would need a per user setting that we store or something like this.
Yeah, I see the point. I wondered whether it would be an option to add the preview as a separate node to markdown. There's e.g. junkawa/markdown-it-link-preview. The preview would be auto-generated when inserting an URL, but once it got removed manually it would stay away persistently in the document. What do you think about it?
And we also talked about showing both vs only showing the preview, and came to the same conclusion as you with the editing issue. In a future version, we could make it so that only the preview is shown, and it has a 3-dot menu for "Edit link" and "Hide preview"
That's a great idea!
/backport to stable25
I could not reproduce the failures reliably locally and they seem to be unrelated, so I'd consider failing cypress tests (outside of the link related ones) as not blocking for the merge.
/compile
The backport to stable25 failed. Please do this backport manually.