Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement simple link decoration in richtext fields #4397

Merged
merged 10 commits into from Jan 9, 2024

Conversation

manuelmeister
Copy link
Member

@manuelmeister manuelmeister commented Jan 6, 2024

It has been requested several times to be able to click links. To reduce this pain we provide a simple prosemirror plugin that highlights all links automatically. The link text can't be changed to something more readable and the link as an element is not saved to the database (doesn't modify the editor storage)
This is only a small step towards an eventual smartlink, but it is the minimal valuable product.

This uses the linkify-it package used by markdown-it maintainers. It also provides support for fuzzy links (e.g. without http://) An example can be seen here: https://markdown-it.github.io/linkify-it/

We only support http and 'https' protocols and fuzzy links with a known tld (.ch,.com,.org,…) or then the link needs to have a port (localhost:3000)

The link is rendered as long as it is detected as a link and has no marks (e.g. bold). The url can only be changed with the keyboard, otherwise if you [ ctrl | cmd | dbl ]-click on the link, it opens the link.

Relates to #4241

@manuelmeister manuelmeister added Feature request type: Frontend deploy! Creates a feature branch deployment for this PR External feedback This Issue is Based on Feedback from the Community labels Jan 6, 2024
Copy link

github-actions bot commented Jan 6, 2024

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

@manuelmeister manuelmeister added deploy! Creates a feature branch deployment for this PR and removed deploy! Creates a feature branch deployment for this PR labels Jan 6, 2024
Copy link
Member

@carlobeltrame carlobeltrame left a comment

Choose a reason for hiding this comment

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

Code LGTM, great stuff! Will test manually later.

rel: 'noopener noreferrer',
}
if (this.editor.isEditable) {
attrs.onclick = `(event.metaKey || event.ctrlKey) && window.open("${link}", "_blank");`
Copy link
Member

Choose a reason for hiding this comment

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

Two inputs:

  • I like the idea that you have to use Ctrl + Click to follow the link, but I'm unsure if all users will discover this themselves. Let's see (I would leave it like this for now)

  • Ctrl + Click is already used by Tiptap. The text-edit dialog (bold, italic) is then displayed. Maybe you can stop this with event.stopPropagation().

Copy link
Member Author

Choose a reason for hiding this comment

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

  • I like the idea that you have to use Ctrl + Click to follow the link, but I'm unsure if all users will discover this themselves. Let's see (I would leave it like this for now)

Sadly the browser prevents the click if it is inside a contenteditable. That's why I've added the click handler. I've also made it possible to doubleclick the link to open it.

  • Ctrl + Click is already used by Tiptap. The text-edit dialog (bold, italic) is then displayed. Maybe you can stop this with event.stopPropagation().

I was able to prevent the bubble menu if you click or select within/whole link.

Copy link
Contributor

@BacLuc BacLuc left a comment

Choose a reason for hiding this comment

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

In my tests the tiptap menu still opens on ctrl + click.
I would say we improve that in the next iterations.
very cool

# Conflicts:
#	frontend/package-lock.json
#	frontend/package.json
@manuelmeister manuelmeister added this pull request to the merge queue Jan 9, 2024
Merged via the queue into ecamp:devel with commit 18cf901 Jan 9, 2024
32 checks passed
@manuelmeister manuelmeister deleted the feature/simple-link branch January 9, 2024 19:04
Copy link

sentry-io bot commented Jan 10, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: undefined is not an object (evaluating 'this.$refs.bubbleMenu.$el') <object>.shouldShow(src/components/form/tiptap/... View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy! Creates a feature branch deployment for this PR External feedback This Issue is Based on Feedback from the Community Feature request type: Frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants