Feature: Optimize theme switcher performance (#4124)

Because: * There is a noticeable delay between switching the theme and the theme icon updating * Related to: #4044 This commit: * Toggles the theme class in the same turbo stream request we use to update the theme icon. * Adds a custom turbo stream action to toggle classes on elements Before: https://github.com/TheOdinProject/theodinproject/assets/7963776/6870d2a3-27af-413a-9df8-3adca85d6c4d After: https://github.com/TheOdinProject/theodinproject/assets/7963776/28cbb153-2348-4976-b11a-8245d85143fd
TheOdinProject · Sep 11, 2023 · 1e0b7b1 · 1e0b7b1
1 parent f8c0d4f
commit 1e0b7b1
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 36 deletions.
diff --git a/app/components/theme/switcher_component.html.erb b/app/components/theme/switcher_component.html.erb
@@ -1,15 +1,4 @@
-<%= link_to(
-      themes_path(theme: other_theme.name),
-      class: yass(link: type),
-      data: {
-        'turbo-method' => :put,
-        'turbo-frame' => '_top',
-        controller: 'theme--switcher',
-        'theme--switcher-current-theme-value' => current_theme.name,
-        'theme--switcher-other-theme-value' => other_theme.name,
-        action: 'click->theme--switcher#toggle'
-      }
-    ) do %>
+<%= link_to themes_path(theme: other_theme.name), class: yass(link: type), data: { turbo_method: :put } do %>
   <%= inline_svg_tag "icons/#{current_theme.icon}.svg", class: yass(icon: type), aria: true, title: 'theme icon' %>
   <%= text unless icon_only? %>
 <% end %>
diff --git a/app/components/theme/switcher_controller.js b/app/components/theme/switcher_controller.js
diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb
@@ -1,16 +1,24 @@
 class ThemesController < ApplicationController
+  before_action :cache_previous_theme
+
   def update
-    theme = params[:theme]
+    @new_theme = Users::Theme.for(params[:theme])
 
-    if Users::Theme.exists?(theme)
-      change_current_theme(theme)
+    respond_to do |format|
+      if @new_theme.present?
+        change_current_theme(params[:theme])
 
-      respond_to do |format|
-        format.html { redirect_back(fallback_location: root_path) }
         format.turbo_stream
+      else
+        flash.now[:alert] = "Sorry, we can't find that theme."
+        format.turbo_stream { render turbo_stream: turbo_stream.update('flash-messages', partial: 'shared/flash') }
       end
-    else
-      redirect_back(fallback_location: root_path, alert: "Sorry, we can't find that theme.", status: :see_other)
     end
   end
+
+  private
+
+  def cache_previous_theme
+    @previous_theme = current_theme
+  end
 end
diff --git a/app/javascript/application.js b/app/javascript/application.js
@@ -22,5 +22,6 @@ import '@fortawesome/fontawesome-free/css/all.css';
 import './src/js/axiosWithCsrf';
 import 'controllers';
 import '@hotwired/turbo-rails';
+import './src/custom_turbo_stream_actions';
 
 Rails.start();
diff --git a/app/javascript/src/custom_turbo_stream_actions.js b/app/javascript/src/custom_turbo_stream_actions.js
@@ -0,0 +1,11 @@
+import { Turbo } from '@hotwired/turbo-rails';
+
+Turbo.StreamActions.toggle_classes = function toggleClasses() {
+  const cssClasses = this.getAttribute('classes').split(' ');
+  const id = this.getAttribute('id');
+  const element = document.getElementById(id);
+
+  cssClasses.forEach((cssClass) => {
+    element.classList.toggle(cssClass);
+  });
+};
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
@@ -1,5 +1,5 @@
 <!DOCTYPE html>
-<html lang="en" class="<%= current_theme.name %>">
+<html lang="en" id="root-element" class="<%= current_theme.name %>">
   <head>
     <% content_for :error_tracking do %>
       <%= render 'layouts/error_tracking' %>

diff --git a/app/views/themes/update.turbo_stream.erb b/app/views/themes/update.turbo_stream.erb
@@ -1,7 +1,10 @@
+
+<%= turbo_stream_action_tag 'toggle_classes', id: 'root-element', classes: [@previous_theme, @new_theme] %>
+
 <%= turbo_stream.update 'theme_switcher' do %>
-  <%= render Theme::SwitcherComponent.new(current_theme:, type: :icon_only) %>
+  <%= render Theme::SwitcherComponent.new(current_theme: @new_theme, type: :icon_only) %>
 <% end %>
 
 <%= turbo_stream.update 'theme_switcher_mobile' do %>
-  <%= render Theme::SwitcherComponent.new(current_theme:, type: :mobile) %>
+  <%= render Theme::SwitcherComponent.new(current_theme: @new_theme, type: :mobile) %>
 <% end %>
diff --git a/package.json b/package.json
@@ -3,7 +3,7 @@
   "version": "1.0.0",
   "private": true,
   "scripts": {
-    "eslint": "eslint --max-warnings 0 './app/javascript/**/*.js' './app/components/**/*.js'",
+    "eslint": "eslint --max-warnings 0 './app/javascript/**/*.js'",
     "build:css": "npm-run-all --parallel \"build:css:* {@}\" --",
     "build:css:application": "postcss ./app/assets/stylesheets/application.postcss.css -o ./app/assets/builds/application.css",
     "build:css:active_admin": "sass ./app/assets/stylesheets/active_admin.scss ./app/assets/builds/active_admin.css --no-source-map --load-path=node_modules --style=compressed"

diff --git a/spec/requests/themes_spec.rb b/spec/requests/themes_spec.rb
@@ -0,0 +1,25 @@
+require 'rails_helper'
+
+RSpec.describe 'Themes' do
+  describe 'PUT #update' do
+    before do
+      cookies[:theme] = 'light'
+    end
+
+    context 'when the theme is valid' do
+      it 'switches themes' do
+        expect do
+          put themes_path(format: :turbo_stream, params: { theme: 'dark' })
+        end.to change { cookies[:theme] }.from('light').to('dark')
+      end
+    end
+
+    context 'when the theme is invalid' do
+      it "doesn't switch themes" do
+        expect do
+          put themes_path(format: :turbo_stream, params: { theme: 'invalid' })
+        end.not_to change { cookies[:theme] }
+      end
+    end
+  end
+end