Merge branch 'fix-theme-switcher' into 'master'

Fix theme selector for light themes

See merge request gitlab-org/gitlab!43239

app/helpers/preferences_helper.rb

@@ -61,8 +61,8 @@ module PreferencesHelper
     @user_application_theme ||= Gitlab::Themes.for_user(current_user).css_class
   end
 
-  def user_application_theme_name
-    @user_application_theme_name ||= Gitlab::Themes.for_user(current_user).name.downcase.tr(' ', '_')
+  def user_application_theme_css_filename
+    @user_application_theme_css_filename ||= Gitlab::Themes.for_user(current_user).css_filename
   end
 
   def user_color_scheme

app/views/layouts/_head.html.haml

@@ -53,7 +53,7 @@
   - else
     = stylesheet_link_tag_defer "application"
   - unless use_startup_css?
-    = stylesheet_link_tag_defer "themes/theme_#{user_application_theme_name}"
+    = stylesheet_link_tag_defer "themes/#{user_application_theme_css_filename}" if user_application_theme_css_filename
   = stylesheet_link_tag "disable_animations", media: "all" if Rails.env.test? || Gitlab.config.gitlab['disable_animations']
   = stylesheet_link_tag_defer 'performance_bar' if performance_bar_enabled?
 

app/views/layouts/_startup_css.haml

@@ -3,5 +3,5 @@
 - startup_filename = current_path?("sessions#new") ? 'signin' : user_application_theme == 'gl-dark' ? 'dark' : 'general'
 
 %style{ type: "text/css" }
-  = Rails.application.assets_manifest.find_sources("themes/theme_#{user_application_theme_name}.css").first.to_s.html_safe
+  = Rails.application.assets_manifest.find_sources("themes/#{user_application_theme_css_filename}.css").first.to_s.html_safe if user_application_theme_css_filename
   = Rails.application.assets_manifest.find_sources("startup/startup-#{startup_filename}.css").first.to_s.html_safe

app/views/profiles/preferences/show.html.haml

@@ -2,7 +2,7 @@
 - @content_class = "limit-container-width" unless fluid_layout
 
 - Gitlab::Themes.each do |theme|
-  = stylesheet_link_tag "themes/theme_#{theme.css_class.gsub('ui-', '')}"
+  = stylesheet_link_tag "themes/#{theme.css_filename}" if theme.css_filename
 
 = form_for @user, url: profile_preferences_path, remote: true, method: :put, html: { class: 'row gl-mt-3 js-preferences-form' } do |f|
   .col-lg-4.application-theme#navigation-theme

changelogs/unreleased/fix-theme-switcher.yml

+---
+title: Fix theme selector not working immediately for some themes
+merge_request: 43239
+author:
+type: fixed

lib/gitlab/themes.rb

@@ -10,21 +10,21 @@ module Gitlab
     APPLICATION_DEFAULT = 1
 
     # Struct class representing a single Theme
-    Theme = Struct.new(:id, :name, :css_class)
+    Theme = Struct.new(:id, :name, :css_class, :css_filename)
 
     # All available Themes
     THEMES = [
-      Theme.new(1, 'Indigo', 'ui-indigo'),
-      Theme.new(6, 'Light Indigo', 'ui-light-indigo'),
-      Theme.new(4, 'Blue', 'ui-blue'),
-      Theme.new(7, 'Light Blue', 'ui-light-blue'),
-      Theme.new(5, 'Green', 'ui-green'),
-      Theme.new(8, 'Light Green', 'ui-light-green'),
-      Theme.new(9, 'Red', 'ui-red'),
-      Theme.new(10, 'Light Red', 'ui-light-red'),
-      Theme.new(2, 'Dark', 'ui-dark'),
-      Theme.new(3, 'Light', 'ui-light'),
-      Theme.new(11, 'Dark Mode (alpha)', 'gl-dark')
+      Theme.new(1, 'Indigo', 'ui-indigo', 'theme_indigo'),
+      Theme.new(6, 'Light Indigo', 'ui-light-indigo', 'theme_light_indigo'),
+      Theme.new(4, 'Blue', 'ui-blue', 'theme_blue'),
+      Theme.new(7, 'Light Blue', 'ui-light-blue', 'theme_light_blue'),
+      Theme.new(5, 'Green', 'ui-green', 'theme_green'),
+      Theme.new(8, 'Light Green', 'ui-light-green', 'theme_light_green'),
+      Theme.new(9, 'Red', 'ui-red', 'theme_red'),
+      Theme.new(10, 'Light Red', 'ui-light-red', 'theme_light_red'),
+      Theme.new(2, 'Dark', 'ui-dark', 'theme_dark'),
+      Theme.new(3, 'Light', 'ui-light', 'theme_light'),
+      Theme.new(11, 'Dark Mode (alpha)', 'gl-dark', nil)
     ].freeze
 
     # Convenience method to get a space-separated String of all the theme

spec/lib/gitlab/themes_spec.rb

@@ -47,4 +47,18 @@ RSpec.describe Gitlab::Themes, lib: true do
       expect(ids).not_to be_empty
     end
   end
+
+  describe 'theme.css_filename' do
+    described_class.each do |theme|
+      next unless theme.css_filename
+
+      context "for #{theme.name}" do
+        it 'returns an existing CSS filename' do
+          css_file_path = Rails.root.join('app/assets/stylesheets/themes', theme.css_filename + '.scss')
+
+          expect(File.exist?(css_file_path)).to eq(true)
+        end
+      end
+    end
+  end
 end

spec/views/profiles/preferences/show.html.haml_spec.rb

@@ -20,6 +20,14 @@ RSpec.describe 'profiles/preferences/show' do
     it 'has an id for anchoring' do
       expect(rendered).to have_css('#navigation-theme')
     end
+
+    it 'has correct stylesheet tags' do
+      Gitlab::Themes.each do |theme|
+        next unless theme.css_filename
+
+        expect(rendered).to have_selector("link[href*=\"themes/#{theme.css_filename}\"]", visible: false)
+      end
+    end
   end
 
   context 'syntax highlighting theme' do