Fix Codex's code review comments

app/assets/stylesheets/embeds.css

@@ -15,7 +15,7 @@
       flex-direction: row;
     }
 
-    .cf-twitter-avatar & {
+    &.og-embed--twitter-avatar {
       flex-direction: row;
     }
   }
@@ -68,7 +68,7 @@
       }
     }
 
-    .cf-twitter-avatar & {
+    .og-embed--twitter-avatar & {
       aspect-ratio: 1;
       border-radius: 50%;
       inline-size: 2lh;

app/helpers/content_filters.rb

@@ -9,5 +9,5 @@ module ContentFilters
   EDITOR_FORMATTING_TAGS = %w[ s u mark ]
   EDITOR_FORMATTING_ATTRIBUTES = %w[ data-language ]
 
-  TextMessagePresentationFilters = ActionText::Content::Filters.new(RemoveSoloUnfurledLinkText, StyleUnfurledTwitterAvatars, SanitizeTags)
+  TextMessagePresentationFilters = ActionText::Content::Filters.new(RemoveSoloUnfurledLinkText, SanitizeTags)
 end

app/helpers/content_filters/style_unfurled_twitter_avatars.rb

@@ -1,26 +0,0 @@
-class ContentFilters::StyleUnfurledTwitterAvatars < ActionText::Content::Filter
-  def applicable?
-    unfurled_twitter_avatars.present?
-  end
-
-  def apply
-    fragment.update do |source|
-      div = source.at_css("div")
-      div["class"] = UNFURLED_TWITTER_AVATAR_CSS_CLASS
-    end
-  end
-
-  private
-    UNFURLED_TWITTER_AVATAR_CSS_CLASS = "cf-twitter-avatar"
-    TWITTER_AVATAR_URL_PREFIX = "https://pbs.twimg.com/profile_images"
-
-    def unfurled_twitter_avatars
-      fragment.find_all(opengraph_css_selector).select do |node|
-        ActionText::Attachment::OpengraphEmbed.from_node(node)&.url.to_s.start_with?(TWITTER_AVATAR_URL_PREFIX)
-      end
-    end
-
-    def opengraph_css_selector
-      "action-text-attachment[@content-type='#{ActionText::Attachment::OpengraphEmbed::OPENGRAPH_EMBED_CONTENT_TYPE}']"
-    end
-end

app/javascript/controllers/unfurl_controller.js

@@ -5,7 +5,7 @@ import { escapeHTML } from "helpers/dom_helpers"
 
 const OPENGRAPH_EMBED_CONTENT_TYPE = "application/vnd.actiontext.opengraph-embed"
 
-const UNFURLED_TWITTER_AVATAR_CSS_CLASS = "cf-twitter-avatar"
+const UNFURLED_TWITTER_AVATAR_CSS_CLASS = "og-embed--twitter-avatar"
 const TWITTER_AVATAR_URL_PREFIX = "https://pbs.twimg.com/profile_images"
 
 export default class extends Controller {
@@ -50,8 +50,8 @@ export default class extends Controller {
   }
 
   #opengraphEmbedHTML({ title, href, image, description }) {
-    return `<actiontext-opengraph-embed class="${this.#embedClass(image)}">
-      <div class="og-embed gap">
+    return `<actiontext-opengraph-embed>
+      <div class="og-embed gap ${this.#embedClass(image)}">
         <div class="og-embed__content">
           <div class="og-embed__title">
             <a href="${escapeHTML(href)}" rel="noreferrer" target="_blank">${escapeHTML(truncateString(title, 280))}</a>

app/javascript/lib/rich_text/campfire_extension.js

@@ -9,6 +9,7 @@ export default class CampfireRichTextExtension extends Lexxy.Extension {
       "figcaption",
       "actiontext-opengraph-embed",
       { tag: "div", attributes: [ "sgid" ] },
+      { tag: "span", attributes: [ "sgid" ] },
       { tag: "img", attributes: [ "alt" ] },
       { tag: "a", attributes: [ "rel", "target" ] }
     ]

app/views/action_text/attachables/_opengraph_embed.html.erb

@@ -1,6 +1,6 @@
 <figure class="attachment attachment--content attachment--og">
   <actiontext-opengraph-embed>
-    <div class="og-embed gap">
+    <div class="og-embed gap <%= "og-embed--twitter-avatar" if opengraph_embed.twitter_avatar? %>">
       <div class="og-embed__content">
         <div class="og-embed__title">
           <%= link_to truncate(opengraph_embed.filename, length: 280, omission: "…"), opengraph_embed.href, rel: "noreferrer", target: "_blank" %>

lib/rails_ext/actiontext_opengraph_embeds.rb

@@ -2,6 +2,7 @@ class ActionText::Attachment::OpengraphEmbed
   include ActiveModel::Model
 
   OPENGRAPH_EMBED_CONTENT_TYPE = "application/vnd.actiontext.opengraph-embed"
+  TWITTER_AVATAR_URL_PREFIX = "https://pbs.twimg.com/profile_images"
 
   class << self
     def from_node(node)
@@ -47,6 +48,10 @@ class ActionText::Attachment::OpengraphEmbed
 
   attr_accessor :href, :url, :filename, :description
 
+  def twitter_avatar?
+    url.to_s.start_with?(TWITTER_AVATAR_URL_PREFIX)
+  end
+
   def attachable_content_type
     OPENGRAPH_EMBED_CONTENT_TYPE
   end

test/helpers/content_filters_test.rb

@@ -30,20 +30,26 @@ class ContentFiltersTest < ActionView::TestCase
     assert_match %r{<div>Hello https://basecamp\.com/<action-text-attachment}, filtered.to_html
   end
 
-  test "unfurled tweet without any image" do
-    text = "<div>https://twitter.com/37signals/status/1750290547908952568<action-text-attachment content-type=\"application/vnd.actiontext.opengraph-embed\" url=\"https://pbs.twimg.com/profile_images/1671940407633010689/9P5gi6LF_200x200.jpg\" href=\"https://twitter.com/37signals/status/1750290547908952568\" filename=\"37signals (@37signals)\" caption=\"We're back up on all apps, everyone. Really sorry for the disruption to your day.\" content=\"<actiontext-opengraph-embed>\n      <div class=&quot;og-embed&quot;>\n        <div class=&quot;og-embed__content&quot;>\n          <div class=&quot;og-embed__title&quot;>37signals (@37signals)</div>\n          <div class=&quot;og-embed__description&quot;>We're back up on all apps, everyone. Really sorry for the disruption to your day.</div>\n        </div>\n        <div class=&quot;og-embed__image&quot;>\n          <img src=&quot;https://pbs.twimg.com/profile_images/1671940407633010689/9P5gi6LF_200x200.jpg&quot; class=&quot;image&quot; alt=&quot;&quot; />\n        </div>\n      </div>\n    </actiontext-opengraph-embed>\"></action-text-attachment></div>"
-    message = Message.create! room: rooms(:pets), body: unfurled_message_body_for_basecamp(text), client_message_id: "0015", creator: users(:jason)
+  test "unfurled tweet with an avatar image gets the twitter avatar treatment" do
+    body = %(<div>https://twitter.com/37signals/status/1750290547908952568<action-text-attachment content-type="application/vnd.actiontext.opengraph-embed" url="https://pbs.twimg.com/profile_images/1671940407633010689/9P5gi6LF_200x200.jpg" href="https://twitter.com/37signals/status/1750290547908952568" filename="37signals (@37signals)" caption="We're back up on all apps, everyone."></action-text-attachment></div>)
+    message = Message.create! room: rooms(:pets), body: body, client_message_id: "0015", creator: users(:jason)
 
-    filtered = ContentFilters::StyleUnfurledTwitterAvatars.apply(message.body.body)
-    assert_match %r{<div class="cf-twitter-avatar">}, filtered.to_html
+    assert_match /og-embed--twitter-avatar/, message_presentation(message)
   end
 
-  test "unfurled tweet containing an image" do
-    text = "<div>https://twitter.com/dhh/status/1748445489648050505<action-text-attachment content-type=\"application/vnd.actiontext.opengraph-embed\" url=\"https://pbs.twimg.com/media/GEO5l04bsAA9f6H.jpg\" href=\"https://twitter.com/dhh/status/1748445489648050505\" filename=\"DHH (@dhh)\" caption=\"We pay homage to the glorious MIT License with the ONCE license. May all our future legalese be as succinct!\" content=\"<actiontext-opengraph-embed>\n      <div class=&quot;og-embed&quot;>\n        <div class=&quot;og-embed__content&quot;>\n          <div class=&quot;og-embed__title&quot;>DHH (@dhh)</div>\n          <div class=&quot;og-embed__description&quot;>We pay homage to the glorious MIT License with the ONCE license. May all our future legalese be as succinct!</div>\n        </div>\n        <div class=&quot;og-embed__image&quot;>\n          <img src=&quot;https://pbs.twimg.com/media/GEO5l04bsAA9f6H.jpg&quot; class=&quot;image&quot; alt=&quot;&quot; />\n        </div>\n      </div>\n    </actiontext-opengraph-embed>\"></action-text-attachment></div>"
-    message = Message.create! room: rooms(:pets), body: unfurled_message_body_for_basecamp(text), client_message_id: "0015", creator: users(:jason)
+  test "unfurled tweet with an avatar image in a lexxy body gets the twitter avatar treatment" do
+    content = %(<actiontext-opengraph-embed><div class="og-embed gap"><div class="og-embed__content"><div class="og-embed__title"><a href="https://twitter.com/x/status/1">Tweet</a></div><div class="og-embed__description">desc</div></div><div class="og-embed__image"><img src="https://pbs.twimg.com/profile_images/x.jpg" class="image center" alt="" /></div></div></actiontext-opengraph-embed>)
+    body = %(<p><a href="https://twitter.com/x/status/1">https://twitter.com/x/status/1</a></p><action-text-attachment content-type="application/vnd.actiontext.opengraph-embed" content="#{CGI.escapeHTML(content)}"></action-text-attachment>)
+    message = Message.create! room: rooms(:pets), body: body, client_message_id: "0015", creator: users(:jason)
 
-    filtered = ContentFilters::StyleUnfurledTwitterAvatars.apply(message.body.body)
-    assert_no_match %r{<div class="cf-twitter-avatar">}, filtered.to_html
+    assert_match /og-embed--twitter-avatar/, message_presentation(message)
+  end
+
+  test "unfurled tweet with a content image is not styled as an avatar" do
+    body = %(<div>https://twitter.com/dhh/status/1748445489648050505<action-text-attachment content-type="application/vnd.actiontext.opengraph-embed" url="https://pbs.twimg.com/media/GEO5l04bsAA9f6H.jpg" href="https://twitter.com/dhh/status/1748445489648050505" filename="DHH (@dhh)" caption="We pay homage to the glorious MIT License!"></action-text-attachment></div>)
+    message = Message.create! room: rooms(:pets), body: body, client_message_id: "0015", creator: users(:jason)
+
+    assert_no_match /og-embed--twitter-avatar/, message_presentation(message)
   end
 
   test "entire message contains an unfurled URL from x.com but unfurls to twitter.com" do