UX: Fix chat separator alignment (#20669)

Also: work around 1px svg shift in scroll-to-bottom button

lib/svg_sprite.rb

@@ -243,37 +243,22 @@ module SvgSprite
     badge_icons
   end
 
-  def self.symbols_for(svg_filename, sprite, strict:)
-    if strict
-      Nokogiri.XML(sprite) { |config| config.options = Nokogiri::XML::ParseOptions::NOBLANKS }
-    else
-      Nokogiri.XML(sprite)
-    end.css("symbol")
-      .filter_map do |sym|
-        icon_id = prepare_symbol(sym, svg_filename)
-        if icon_id.present?
-          sym.attributes["id"].value = icon_id
-          sym.css("title").each(&:remove)
-          [icon_id, sym.to_xml]
+  def self.core_svg_sprites
+    @core_svg_sprites ||=
+      begin
+        CORE_SVG_SPRITES.map do |path|
+          { filename: File.basename(path, ".svg"), sprite: File.read(path) }
         end
       end
-      .to_h
-  end
-
-  def self.core_svgs
-    @core_svgs ||=
-      CORE_SVG_SPRITES.reduce({}) do |symbols, path|
-        symbols.merge!(symbols_for(File.basename(path, ".svg"), File.read(path), strict: true))
-      end
   end
 
   # Just used in tests
   def self.clear_plugin_svg_sprite_cache!
-    @plugin_svgs = nil
+    @plugin_svg_sprites = nil
   end
 
-  def self.plugin_svgs
-    @plugin_svgs ||=
+  def self.plugin_svg_sprites
+    @plugin_svg_sprites ||=
       begin
         plugin_paths = []
         Discourse
@@ -283,13 +268,13 @@ module SvgSprite
 
         custom_sprite_paths = Dir.glob(plugin_paths)
 
-        custom_sprite_paths.reduce({}) do |symbols, path|
-          symbols.merge!(symbols_for(File.basename(path, ".svg"), File.read(path), strict: true))
+        custom_sprite_paths.map do |path|
+          { filename: File.basename(path, ".svg"), sprite: File.read(path) }
         end
       end
   end
 
-  def self.theme_svgs(theme_id)
+  def self.theme_svg_sprites(theme_id)
     if theme_id.present?
       theme_ids = Theme.transform_ids(theme_id)
 
@@ -310,25 +295,17 @@ module SvgSprite
           )
         end
 
-        theme_sprites.reduce({}) do |symbols, (upload_id, sprite)|
-          begin
-            symbols.merge!(symbols_for("theme_#{theme_id}_#{upload_id}.svg", sprite, strict: false))
-          rescue => e
-            Rails.logger.warn(
-              "Bad XML in custom sprite in theme with ID=#{theme_id}. Error info: #{e.inspect}",
-            )
-          end
-
-          symbols
+        theme_sprites.map do |upload_id, sprite|
+          { filename: "theme_#{theme_id}_#{upload_id}.svg", sprite: sprite }
         end
       end
     else
-      {}
+      []
     end
   end
 
-  def self.custom_svgs(theme_id)
-    plugin_svgs.merge(theme_svgs(theme_id))
+  def self.custom_svg_sprites(theme_id)
+    plugin_svg_sprites + theme_svg_sprites(theme_id)
   end
 
   def self.all_icons(theme_id = nil)
@@ -362,10 +339,34 @@ module SvgSprite
     cache&.clear
   end
 
-  def self.svgs_for(theme_id)
-    svgs = core_svgs
-    svgs = svgs.merge(custom_svgs(theme_id)) if theme_id.present?
-    svgs
+  def self.sprites_for(theme_id)
+    sprites = core_svg_sprites
+    sprites += custom_svg_sprites(theme_id) if theme_id.present?
+    sprites
+  end
+
+  def self.core_svgs
+    @core_svgs ||=
+      begin
+        symbols = {}
+
+        CORE_SVG_SPRITES.each do |filename|
+          svg_filename = "#{File.basename(filename, ".svg")}"
+
+          Nokogiri
+            .XML(File.open(filename)) do |config|
+              config.options = Nokogiri::XML::ParseOptions::NOBLANKS
+            end
+            .css("symbol")
+            .each do |sym|
+              icon_id = prepare_symbol(sym, svg_filename)
+              sym.attributes["id"].value = icon_id
+              symbols[icon_id] = sym.to_xml
+            end
+        end
+
+        symbols
+      end
   end
 
   def self.bundle(theme_id = nil)
@@ -381,8 +382,34 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
 " \
         "".dup
 
-    svg_subset << core_svgs.slice(*icons).values.join
-    svg_subset << custom_svgs(theme_id).values.join
+    core_svgs.each { |icon_id, sym| svg_subset << sym if icons.include?(icon_id) }
+
+    custom_svg_sprites(theme_id).each do |item|
+      begin
+        svg_file =
+          Nokogiri.XML(item[:sprite]) do |config|
+            config.options = Nokogiri::XML::ParseOptions::NOBLANKS
+          end
+      rescue => e
+        Rails.logger.warn(
+          "Bad XML in custom sprite in theme with ID=#{theme_id}. Error info: #{e.inspect}",
+        )
+      end
+
+      next if !svg_file
+
+      svg_file
+        .css("symbol")
+        .each do |sym|
+          icon_id = prepare_symbol(sym, item[:filename])
+
+          if icon_id.present?
+            sym.attributes["id"].value = icon_id
+            sym.css("title").each(&:remove)
+            svg_subset << sym.to_xml
+          end
+        end
+    end
 
     svg_subset << "</svg>"
   end
@@ -390,16 +417,46 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
   def self.search(searched_icon)
     searched_icon = process(searched_icon.dup)
 
-    svgs_for(SiteSetting.default_theme_id)[searched_icon] || false
+    sprites_for(SiteSetting.default_theme_id).each do |item|
+      svg_file = Nokogiri.XML(item[:sprite])
+
+      svg_file
+        .css("symbol")
+        .each do |sym|
+          icon_id = prepare_symbol(sym, item[:filename])
+
+          if searched_icon == icon_id
+            sym.attributes["id"].value = icon_id
+            sym.css("title").each(&:remove)
+            return sym.to_xml
+          end
+        end
+    end
+
+    false
   end
 
   def self.icon_picker_search(keyword, only_available = false)
     icons = all_icons(SiteSetting.default_theme_id) if only_available
+    results = Set.new
 
-    symbols = svgs_for(SiteSetting.default_theme_id)
-    symbols.slice!(*icons) if only_available
-    symbols.reject! { |icon_id, sym| !icon_id.include?(keyword) } unless keyword.empty?
-    symbols.sort_by(&:first).map { |icon_id, symbol| { id: icon_id, symbol: symbol } }
+    sprites_for(SiteSetting.default_theme_id).each do |item|
+      svg_file = Nokogiri.XML(item[:sprite])
+
+      svg_file
+        .css("symbol")
+        .each do |sym|
+          icon_id = prepare_symbol(sym, item[:filename])
+          next if only_available && !icons.include?(icon_id)
+          if keyword.empty? || icon_id.include?(keyword)
+            sym.attributes["id"].value = icon_id
+            sym.css("title").each(&:remove)
+            results.add(id: icon_id, symbol: sym.to_xml)
+          end
+        end
+    end
+
+    results.sort_by { |icon| icon[:id] }
   end
 
   # For use in no_ember .html.erb layouts
@@ -482,7 +539,14 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
 
   def self.custom_icons(theme_id)
     # Automatically register icons in sprites added via themes or plugins
-    custom_svgs(theme_id).keys
+    icons = []
+    custom_svg_sprites(theme_id).each do |item|
+      svg_file = Nokogiri.XML(item[:sprite])
+      svg_file
+        .css("symbol")
+        .each { |sym| icons << sym.attributes["id"].value if sym.attributes["id"].present? }
+    end
+    icons
   end
 
   def self.process(icon_name)

plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js

@@ -613,6 +613,7 @@ export default class ChatLivePane extends Component {
       const message = ChatMessage.create(this.args.channel, data.chat_message);
       this.args.channel.addMessages([message]);
       this.scrollToLatestMessage();
+      this.updateLastReadMessage();
     } else {
       // If we are almost at the bottom, we append the message and notice the user
       const message = ChatMessage.create(this.args.channel, data.chat_message);
@@ -895,16 +896,19 @@ export default class ChatLivePane extends Component {
   @action
   editLastMessageRequested() {
     const lastUserMessage = this.args.channel.messages.findLast(
-      (message) =>
-        message.user.id === this.currentUser.id &&
-        !message.staged &&
-        !message.error
+      (message) => message.user.id === this.currentUser.id
     );
 
-    if (lastUserMessage) {
-      this.editingMessage = lastUserMessage;
-      this._focusComposer();
+    if (!lastUserMessage) {
+      return;
     }
+
+    if (lastUserMessage.staged || lastUserMessage.error) {
+      return;
+    }
+
+    this.editingMessage = lastUserMessage;
+    this._focusComposer();
   }
 
   @action

plugins/chat/assets/stylesheets/common/base-common.scss

@@ -384,6 +384,7 @@ $float-height: 530px;
 
       .d-icon {
         color: var(--secondary);
+        margin-left: 1px; // "fixes" the 1px svg shift
       }
     }
 

plugins/chat/assets/stylesheets/common/chat-message-separator.scss

@@ -56,7 +56,9 @@
     }
 
     .chat-message-separator__text-container {
-      padding-top: 7px;
+      align-items: center;
+      display: flex;
+      height: 40px;
       position: sticky;
       top: -1px;
 
@@ -98,11 +100,7 @@
 
       .chat-message-separator__line {
         border-top: 1px solid var(--secondary-high);
-        left: 0;
         margin: 0 0 -1px;
-        position: relative;
-        right: 0;
-        top: -1px;
       }
     }
   }

plugins/chat/spec/system/shortcuts/chat_composer_spec.rb

@@ -70,5 +70,19 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do
 
       expect(page.find(".chat-composer-message-details")).to have_content(message_1.message)
     end
+
+    context "when last message is not editable" do
+      after { page.driver.browser.network_conditions = { offline: false } }
+
+      it "does not edit a message" do
+        chat.visit_channel(channel_1)
+        page.driver.browser.network_conditions = { offline: true }
+        channel_page.send_message("Hello world")
+
+        find(".chat-composer-input").send_keys(:arrow_up)
+
+        expect(page).to have_no_css(".chat-composer-message-details")
+      end
+    end
   end
 end

spec/lib/svg_sprite/svg_sprite_spec.rb

@@ -184,7 +184,7 @@ RSpec.describe SvgSprite do
       )
       theme.save!
 
-      sprite_files = SvgSprite.custom_svgs(theme.id).values.join("|")
+      sprite_files = SvgSprite.custom_svg_sprites(theme.id).join("|")
       expect(sprite_files).to match(/my-custom-theme-icon/)
 
       SvgSprite.bundle(theme.id)
@@ -230,9 +230,9 @@ RSpec.describe SvgSprite do
     expect(SvgSprite.bundle).to match(/far-building/)
   end
 
-  describe "#custom_svgs" do
+  describe "#custom_svg_sprites" do
     it "is empty by default" do
-      expect(SvgSprite.custom_svgs(nil)).to be_empty
+      expect(SvgSprite.custom_svg_sprites(nil)).to be_empty
       expect(SvgSprite.bundle).not_to be_empty
     end
 
@@ -254,7 +254,7 @@ RSpec.describe SvgSprite do
       end
 
       it "includes custom icons from plugins" do
-        expect(SvgSprite.custom_svgs(nil).size).to eq(1)
+        expect(SvgSprite.custom_svg_sprites(nil).size).to eq(1)
         expect(SvgSprite.bundle).to match(/custom-icon/)
       end
     end

spec/models/theme_field_spec.rb

@@ -612,7 +612,7 @@ HTML
 
   describe "SVG sprite theme fields" do
     let :svg_content do
-      "<svg><symbol id='test'></symbol></svg>"
+      "<svg></svg>"
     end
 
     let :upload_file do
@@ -651,10 +651,10 @@ HTML
 
     it "clears SVG sprite cache when upload is deleted" do
       theme_field
-      expect(SvgSprite.custom_svgs(theme.id).size).to eq(1)
+      expect(SvgSprite.custom_svg_sprites(theme.id).size).to eq(1)
 
       theme_field.destroy!
-      expect(SvgSprite.custom_svgs(theme.id).size).to eq(0)
+      expect(SvgSprite.custom_svg_sprites(theme.id).size).to eq(0)
     end
 
     it "crashes gracefully when svg is invalid" do