Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions app/helpers/html_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
module HtmlHelper
def format_html(html)
fragment = Nokogiri::HTML.fragment(html)

auto_link(fragment)

fragment.to_html.html_safe
end

private
EXCLUDED_ELEMENTS = %w[ a figcaption pre code ]
EMAIL_REGEXP = /\b[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\b/
URL_REGEXP = URI::DEFAULT_PARSER.make_regexp(%w[http https])

def auto_link(fragment)
fragment.traverse do |node|
next unless auto_linkable_node?(node)

content = node.text
linked_content = content.dup

auto_link_urls(linked_content)
auto_link_emails(linked_content)

if linked_content != content
node.replace(Nokogiri::HTML.fragment(linked_content))
end
end
end

def auto_linkable_node?(node)
node.text? && node.ancestors.none? { |ancestor| EXCLUDED_ELEMENTS.include?(ancestor.name) }
end

def auto_link_urls(linked_content)
linked_content.gsub!(URL_REGEXP) do |match|
%(<a href="#{match}" rel="noreferrer">#{match}</a>)
end
end

def auto_link_emails(text)
text.gsub!(EMAIL_REGEXP) do |match|
%(<a href="mailto:#{match}">#{match}</a>)
end
end
end
15 changes: 15 additions & 0 deletions app/javascript/controllers/retarget_links_controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { Controller } from "@hotwired/stimulus"

export default class extends Controller {
connect() {
this.element.querySelectorAll("a").forEach(this.#retargetLink.bind(this))

@jorgemanrubia jorgemanrubia Jun 2, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added this to fix a problem where clicking on internal links will fail with a turbo-frame missing error. This will also make external links open in new pages.

I didn't handle this on the server because determining what's an internal link is simpler in the client, as right now we don't have a good way of querying for the current host in the server (in a way that doesn't require a web request/controller context). I wasn't sure if we could just use that %{tenant} variable I'm seeing we use for config.action_mailer.default_url_options in regular hosts. Like, we could define config.hosts and assume that the first config.host is the app URL (as we did in KIA), but I wasn't sure how to handle the tenant thing. cc @flavorjones

Regardless of that, I think doing this client side is totally fine. We do something like this in campfire too.

@flavorjones flavorjones Jun 2, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FWIW we're planning to move away from using subdomains as the tenant parameter, to using path-based tenanting (similar to BC4's account slug), in which case the hostname will be the same for all tenants.

}

#retargetLink(link) {
link.target = this.#targetsSameDomain(link) ? "_top" : "_blank"
}

#targetsSameDomain(link) {
return link.href.startsWith(window.location.origin)
}
}
2 changes: 1 addition & 1 deletion app/views/cards/comments/_comment.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<% end %>
</div>

<div class="comment__body rich-text-content txt-align-start" data-controller="syntax-highlight">
<div class="comment__body rich-text-content txt-align-start" data-controller="syntax-highlight retarget-links">
<%= comment.body %>
</div>

Expand Down
2 changes: 1 addition & 1 deletion app/views/cards/container/_title.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<h1 class="card__title flex align-start gap-half">
<%= link_to card.title, edit_collection_card_path(card.collection, card), class: "card__title-link" %>
</h1>
<div class="card__description rich-text-content margin-block-half rich-text-content" data-controller="syntax-highlight">
<div class="card__description rich-text-content margin-block-half rich-text-content" data-controller="syntax-highlight retarget-links">
<%= card.description %>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/layouts/action_text/contents/_content.html.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<div class="action-text-content">
<%= yield -%>
<%= format_html yield -%>
</div>
35 changes: 35 additions & 0 deletions test/helpers/html_helper_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
require "test_helper"

class HtmlHelperTest < ActionView::TestCase
test "converts URLs into anchor tags" do
assert_equal_html \
%(<p>Check this: <a href="https://example.com" rel="noreferrer">https://example.com</a></p>),
format_html("<p>Check this: https://example.com</p>")
end

test "respect existing links" do
assert_equal_html \
%(<p>Check this: <a href="https://example.com">https://example.com</a></p>),
format_html("<p>Check this: <a href=\"https://example.com\">https://example.com</a></p>")
end

test "converts email addresses into mailto links" do
assert_equal_html \
%(<p>Contact us at <a href="mailto:support@example.com">support@example.com</a></p>),
format_html("<p>Contact us at support@example.com</p>")
end

test "respect existing linked emails" do
assert_equal_html \
%(<p>Contact us at <a href="mailto:support@example.com">support@example.com</a></p>),
format_html(%(<p>Contact us at <a href="mailto:support@example.com">support@example.com</a></p>))
end

test "don't autolink content in excluded elements" do
%w[ figcaption pre code ].each do |element|
assert_equal_html \
"<#{element}>Check this: https://example.com</#{element}>",
format_html("<#{element}>Check this: https://example.com</#{element}>")
end
end
end