FEATURE: support custom icons in themes (#7155)

approved

#1

FEATURE: support custom icons in themes (#7155)

  • First take

  • Add support for sprites in themes

Automatically register any custom icons added via themes or plugins

  • Fix theme sprite caching

  • Simplify test

  • Update lib/svg_sprite/svg_sprite.rb

Co-Authored-By: pmusaraj pmusaraj@gmail.com

  • Fix /svg-sprite/search request
diff --git a/app/controllers/svg_sprite_controller.rb b/app/controllers/svg_sprite_controller.rb
index 266db14..a3c5cf4 100644
--- a/app/controllers/svg_sprite_controller.rb
+++ b/app/controllers/svg_sprite_controller.rb
@@ -27,13 +27,16 @@ class SvgSpriteController < ApplicationController
   end
 
   def search
-    keyword = params.require(:keyword)
-    data = SvgSprite.search(keyword)
+    RailsMultisite::ConnectionManagement.with_hostname(params[:hostname]) do
+
+      keyword = params.require(:keyword)
+      data = SvgSprite.search(keyword)
 
-    if data.blank?
-      render body: nil, status: 404
-    else
-      render plain: data.inspect, disposition: nil, content_type: 'text/plain'
+      if data.blank?
+        render body: nil, status: 404
+      else
+        render plain: data.inspect, disposition: nil, content_type: 'text/plain'
+      end
     end
   end
 end
diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index a591a6e..5a1e36c 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -9,6 +9,7 @@ class ThemeField < ActiveRecord::Base
 
   after_commit do |field|
     SvgSprite.expire_cache if field.target_id == Theme.targets[:settings]
+    SvgSprite.expire_cache if field.name == SvgSprite.theme_sprite_variable_name
   end
 
   scope :find_by_theme_ids, ->(theme_ids) {
diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb
index f6946b2..2ccb63c 100644
--- a/lib/svg_sprite/svg_sprite.rb
+++ b/lib/svg_sprite/svg_sprite.rb
@@ -189,8 +189,28 @@ module SvgSprite
 
   FA_ICON_MAP = { 'far fa-' => 'far-', 'fab fa-' => 'fab-', 'fas fa-' => '', 'fa-' => '' }
 
-  SVG_SPRITE_PATHS = Dir.glob(["#{Rails.root}/vendor/assets/svg-icons/**/*.svg",
-                               "#{Rails.root}/plugins/*/svg-icons/*.svg"])
+  CORE_SVG_SPRITES = Dir.glob("#{Rails.root}/vendor/assets/svg-icons/**/*.svg")
+
+  THEME_SPRITE_VAR_NAME = "icons-sprite"
+
+  def self.custom_svg_sprites(theme_ids = [])
+    custom_sprite_paths = Dir.glob("#{Rails.root}/plugins/*/svg-icons/*.svg")
+
+    ThemeField.where(type_id: ThemeField.types[:theme_upload_var], name: THEME_SPRITE_VAR_NAME, theme_id: Theme.transform_ids(theme_ids))
+      .pluck(:upload_id).each do |upload_id|
+
+      upload = Upload.find(upload_id)
+      original_path = Discourse.store.path_for(upload)
+      if original_path.blank?
+        external_copy = Discourse.store.download(upload) rescue nil
+        original_path = external_copy.try(:path)
+      end
+
+      custom_sprite_paths << Discourse.store.path_for(upload) if original_path.present?
+    end
+
+    custom_sprite_paths
+  end
 
   def self.all_icons(theme_ids = [])
     get_set_cache("icons_#{Theme.transform_ids(theme_ids).join(',')}") do
@@ -200,6 +220,7 @@ module SvgSprite
         .merge(badge_icons)
         .merge(group_icons)
         .merge(theme_icons(theme_ids))
+        .merge(custom_icons(theme_ids))
         .delete_if { |i| i.blank? || i.include?("/") }
         .map! { |i| process(i.dup) }
         .merge(SVG_ICONS)
@@ -221,11 +242,13 @@ module SvgSprite
     cache&.clear
   end
 
+  def self.sprite_sources(theme_ids)
+    CORE_SVG_SPRITES | custom_svg_sprites(theme_ids)
+  end
+
   def self.bundle(theme_ids = [])
     icons = all_icons(theme_ids)
 
-    doc = File.open("#{Rails.root}/vendor/assets/svg-icons/fontawesome/solid.svg") { |f| Nokogiri::XML(f) }
-
     svg_subset = """<!--
 Discourse SVG subset of Font Awesome Free by @fontawesome - https://fontawesome.com
 License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License)
@@ -233,7 +256,7 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
 <svg xmlns='http://www.w3.org/2000/svg' style='display: none;'>
 """.dup
 
-    SVG_SPRITE_PATHS.each do |fname|
+    sprite_sources(theme_ids).each do |fname|
       svg_file = Nokogiri::XML(File.open(fname)) do |config|
         config.options = Nokogiri::XML::ParseOptions::NOBLANKS
       end
@@ -256,7 +279,7 @@ 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)
 
-    SVG_SPRITE_PATHS.each do |fname|
+    sprite_sources([SiteSetting.default_theme_id]).each do |fname|
       svg_file = Nokogiri::XML(File.open(fname))
       svg_filename = "#{File.basename(fname, ".svg")}"
 
@@ -274,6 +297,10 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
     false
   end
 
+  def self.theme_sprite_variable_name
+    THEME_SPRITE_VAR_NAME
+  end
+
   def self.prepare_symbol(symbol, svg_filename)
     icon_id = symbol.attr('id')
 
@@ -331,6 +358,19 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
     theme_icon_settings
   end
 
+  def self.custom_icons(theme_ids)
+    # Automatically register icons in sprites added via themes or plugins
+    icons = []
+    custom_svg_sprites(theme_ids).each do |fname|
+      svg_file = Nokogiri::XML(File.open(fname))
+
+      svg_file.css('symbol').each do |sym|
+        icons << sym.attributes['id'].value
+      end
+    end
+    icons
+  end
+
   def self.fa4_shim_file
     "#{Rails.root}/lib/svg_sprite/fa4-renames.json"
   end
diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb
index 521b405..6d54ad0 100644
--- a/lib/upload_creator.rb
+++ b/lib/upload_creator.rb
@@ -37,6 +37,7 @@ class UploadCreator
 
       is_image = FileHelper.is_supported_image?(@filename)
       is_image ||= @image_info && FileHelper.is_supported_image?("test.#{@image_info.type}")
+      is_image = false if @opts[:for_theme]
 
       if is_image
         extract_image_info!
diff --git a/spec/components/svg_sprite/svg_sprite_spec.rb b/spec/components/svg_sprite/svg_sprite_spec.rb
index b33fc70..2b6e4bc 100644
--- a/spec/components/svg_sprite/svg_sprite_spec.rb
+++ b/spec/components/svg_sprite/svg_sprite_spec.rb
@@ -99,6 +99,19 @@ describe SvgSprite do
     expect(SvgSprite.all_icons([parent_theme.id])).to include("dragon")
   end
 
+  it 'includes custom icons from a sprite in a theme' do
+    theme = Fabricate(:theme)
+    fname = "custom-theme-icon-sprite.svg"
+
+    upload = UploadCreator.new(file_from_fixtures(fname), fname, for_theme: true).create_for(-1)
+
+    theme.set_field(target: :common, name: SvgSprite.theme_sprite_variable_name, upload_id: upload.id, type: :theme_upload_var)
+    theme.save!
+
+    expect(Upload.where(id: upload.id)).to be_exist
+    expect(SvgSprite.bundle([theme.id])).to match(/my-custom-theme-icon/)
+  end
+
   it 'includes icons from SiteSettings' do
     SiteSetting.svg_icon_subset = "blender|drafting-compass|fab-bandcamp"
 
diff --git a/spec/fixtures/images/custom-theme-icon-sprite.svg b/spec/fixtures/images/custom-theme-icon-sprite.svg
new file mode 100644
index 0000000..23974ec
--- /dev/null
+++ b/spec/fixtures/images/custom-theme-icon-sprite.svg
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg" style="display: none;">
+  <symbol id="my-custom-theme-icon" viewBox="0 0 496 512">
+    <path d="M248 8C111.03 8 0 119.03 0 256s111.03 248 248 248 248-111.03 248-248S384.97 8 248 8zm0 376c-17.67 0-32-14.33-32-32s14.33-32 32-32 32 14.33 32 32-14.33 32-32 32zm0-128c-53.02 0-96 42.98-96 96s42.98 96 96 96c-106.04 0-192-85.96-192-192S141.96 64 248 64c53.02 0 96 42.98 96 96s-42.98 96-96 96zm0-128c-17.67 0-32 14.33-32 32s14.33 32 32 32 32-14.33 32-32-14.33-32-32-32z"></path>
+  </symbol>
+</svg>
diff --git a/spec/requests/svg_sprite_controller_spec.rb b/spec/requests/svg_sprite_controller_spec.rb
index 8eec7bc..cbda1b0 100644
--- a/spec/requests/svg_sprite_controller_spec.rb
+++ b/spec/requests/svg_sprite_controller_spec.rb
@@ -47,5 +47,23 @@ describe SvgSpriteController do
       get "/svg-sprite/search/fa-not-a-valid-icon"
       expect(response.status).to eq(404)
     end
+
+    it "should find a custom icon in default theme" do
+      theme = Fabricate(:theme)

[... diff too long, it was truncated ...]

GitHub sha: d6d4a5ba


Approved #2

#3

This commit has been mentioned on Discourse Meta. There might be relevant details there: