UX: Conditionally refresh page on wizard styling step (#31193)

Previously, were always forcing the page to reload
for the wizard after pressing Next for the styling step,
with the logic that if style changes are being made,
the admin needs to see them straight away.

However this doesn't make sense if nothing changes on
that step. This commit makes the change to only refresh
the page if any of the settings on the step changed,
bringing it in line with other steps.
This commit is contained in:
Martin Brennan
2025-02-06 10:31:22 +10:00
committed by GitHub
parent 1ab5bc2bad
commit 8f72a57363
3 changed files with 46 additions and 4 deletions

View File

@ -265,6 +265,10 @@ class Wizard
scheme_name = ((updater.fields[:color_scheme] || "") || ColorScheme::LIGHT_THEME_ID)
if updater.setting_changed?(:base_font) || updater.setting_changed?(:heading_font)
updater.refresh_required = true
end
next unless scheme_name.present? && ColorScheme.is_base?(scheme_name)
name = I18n.t("color_schemes.#{scheme_name.downcase.gsub(" ", "_")}_theme_name")
@ -273,9 +277,13 @@ class Wizard
scheme ||=
ColorScheme.create_from_base(name: name, via_wizard: true, base_scheme_id: scheme_name)
theme_changed = false
if default_theme
default_theme.color_scheme_id = scheme.id
default_theme.save!
if default_theme.color_scheme_id != scheme.id
default_theme.color_scheme_id = scheme.id
default_theme.save!
theme_changed = true
end
else
theme =
Theme.create!(
@ -285,10 +293,14 @@ class Wizard
)
theme.set_default!
theme_changed = true
end
updater.update_setting(:default_dark_mode_color_scheme_id, -1) if scheme.is_dark?
updater.refresh_required = true
if updater.setting_changed?(:default_dark_mode_color_scheme_id) || theme_changed
updater.refresh_required = true
end
end
end
end

View File

@ -11,6 +11,7 @@ class Wizard
@step = step
@refresh_required = false
@fields = fields
@settings_changed = Set.new
end
def update
@ -30,6 +31,10 @@ class Wizard
@refresh_required
end
def setting_changed?(id)
@settings_changed.include?(id)
end
def update_setting(id, value)
value = value.strip if value.is_a?(String)
@ -37,7 +42,10 @@ class Wizard
value = Upload.get_from_url(value) || ""
end
SiteSetting.set_and_log(id, value, @current_user) if SiteSetting.get(id) != value
if SiteSetting.get(id) != value
SiteSetting.set_and_log(id, value, @current_user)
@settings_changed << id
end
end
def apply_setting(id)

View File

@ -91,6 +91,7 @@ RSpec.describe Wizard::StepUpdater do
)
updater.update
expect(updater.success?).to eq(true)
expect(updater.refresh_required?).to eq(true)
expect(wizard.completed_steps?("styling")).to eq(true)
expect(SiteSetting.base_font).to eq("open_sans")
expect(SiteSetting.heading_font).to eq("oswald")
@ -100,11 +101,32 @@ RSpec.describe Wizard::StepUpdater do
updater = wizard.create_updater("styling", site_font: "open_sans", homepage_style: "latest")
updater.update
expect(updater.success?).to eq(true)
expect(updater.refresh_required?).to eq(true)
expect(wizard.completed_steps?("styling")).to eq(true)
expect(SiteSetting.base_font).to eq("open_sans")
expect(SiteSetting.heading_font).to eq("open_sans")
end
it "does not require refresh if the font, color scheme, or theme are unchanged" do
SiteSetting.base_font = "open_sans"
SiteSetting.heading_font = "open_sans"
SiteSetting.top_menu = "latest|categories|unread|top"
dark_scheme = ColorScheme.find_by(name: "Dark")
Theme.find_default.update!(color_scheme: dark_scheme)
SiteSetting.default_dark_mode_color_scheme_id = -1
updater =
wizard.create_updater(
"styling",
color_scheme: "Dark",
site_font: "open_sans",
heading_font: "open_sans",
homepage_style: "latest",
)
updater.update
expect(updater.success?).to eq(true)
expect(updater.refresh_required?).to eq(false)
end
context "with colors" do
context "with an existing color scheme" do
fab!(:color_scheme) { Fabricate(:color_scheme, name: "existing", via_wizard: true) }