mirror of
https://github.com/discourse/discourse.git
synced 2025-05-29 01:31:35 +08:00
FEATURE: Improve wizard font selection and set Inter as default font for new sites (#30974)
This commit narrows down the list of fonts we offer in our setup wizard and simplifies things to only show a single font dropdown. This selection will then set the `base_font` and `heading_font` site setting to the same value. For existing sites that may have set different values, we will still show 2 dropdowns when visiting the wizard. We are also changing our default font to the more modern selection Inter, replacing Arial. Arial is very dependent on system installed fonts, whereas Inter we can package to everyone in Discourse. Finally, for existing sites that have not changed their default from Arial, we will keep that value via a migration so we do not surprise site owners with a completely new font.
This commit is contained in:
@ -20,7 +20,10 @@ export default class Dropdown extends Component {
|
||||
}
|
||||
}
|
||||
|
||||
if (this.args.field.id === "body_font") {
|
||||
if (
|
||||
this.args.field.id === "body_font" ||
|
||||
this.args.field.id === "site_font"
|
||||
) {
|
||||
for (let choice of this.args.field.choices) {
|
||||
set(choice, "classNames", `body-font-${choice.id.replace(/_/g, "-")}`);
|
||||
}
|
||||
@ -74,6 +77,7 @@ export default class Dropdown extends Component {
|
||||
return ColorPalettes;
|
||||
case "body_font":
|
||||
case "heading_font":
|
||||
case "site_font":
|
||||
return FontSelector;
|
||||
case "homepage_style":
|
||||
return HomepageStyleSelector;
|
||||
|
@ -57,6 +57,7 @@ export default class PreviewBase extends Component {
|
||||
this.step.findField("color_scheme")?.addListener(this.themeChanged);
|
||||
this.step.findField("homepage_style")?.addListener(this.themeChanged);
|
||||
this.step.findField("body_font")?.addListener(this.themeBodyFontChanged);
|
||||
this.step.findField("site_font")?.addListener(this.themeFontChanged);
|
||||
this.step
|
||||
.findField("heading_font")
|
||||
?.addListener(this.themeHeadingFontChanged);
|
||||
@ -74,6 +75,7 @@ export default class PreviewBase extends Component {
|
||||
this.step
|
||||
.findField("body_font")
|
||||
?.removeListener(this.themeBodyFontChanged);
|
||||
this.step.findField("site_font")?.removeListener(this.themeFontChanged);
|
||||
this.step
|
||||
.findField("heading_font")
|
||||
?.removeListener(this.themeHeadingFontChanged);
|
||||
@ -99,6 +101,13 @@ export default class PreviewBase extends Component {
|
||||
}
|
||||
}
|
||||
|
||||
@action
|
||||
themeFontChanged() {
|
||||
if (!this.loadingFontVariants) {
|
||||
this.loadFontVariants(this.wizard.font);
|
||||
}
|
||||
}
|
||||
|
||||
loadFontVariants(font) {
|
||||
if (!font) {
|
||||
return Promise.resolve();
|
||||
|
@ -50,11 +50,17 @@ export default class Wizard {
|
||||
}
|
||||
|
||||
get font() {
|
||||
return this.findStep("styling")?.findField("body_font").chosen;
|
||||
const bodyFont = this.getFont("body");
|
||||
return bodyFont ? bodyFont : this.getFont("site");
|
||||
}
|
||||
|
||||
get headingFont() {
|
||||
return this.findStep("styling")?.findField("heading_font").chosen;
|
||||
const headingFont = this.getFont("heading");
|
||||
return headingFont ? headingFont : this.getFont("site");
|
||||
}
|
||||
|
||||
getFont(type) {
|
||||
return this.findStep("styling")?.findField(`${type}_font`)?.chosen;
|
||||
}
|
||||
|
||||
findStep(id) {
|
||||
|
@ -5428,6 +5428,8 @@ en:
|
||||
label: "Color palette"
|
||||
body_font:
|
||||
label: "Body font"
|
||||
site_font:
|
||||
label: "Font"
|
||||
heading_font:
|
||||
label: "Heading font"
|
||||
styling_preview:
|
||||
|
@ -21,6 +21,12 @@
|
||||
# defined in the choices option. A type:list setting with the word 'colors' in
|
||||
# its name will make color values have a bold line of the corresponding color
|
||||
#
|
||||
# list_type: simple - Allow multiple values to be selected for the list, and new values can be defined if
|
||||
# allow_any is true. This list displays like a list of items with add/remove buttons,
|
||||
# and the ability to reorder items.
|
||||
# list_type: compact - Allow multiple values to be selected for the list, and new values can be defined if
|
||||
# allow_any is true. This list displays like the tag selector.
|
||||
#
|
||||
# type: enum - A single value, chosen from a set of valid values in the choices option.
|
||||
# type: json_schema - The setting value is stored as JSON in the database. Used
|
||||
# in the format `json_schema: ClassName` where the class implements
|
||||
@ -412,7 +418,7 @@ basic:
|
||||
default: ""
|
||||
hidden: true
|
||||
base_font:
|
||||
default: "arial"
|
||||
default: "inter"
|
||||
choices: "BaseFontSetting.values"
|
||||
refresh: true
|
||||
type: list
|
||||
@ -420,7 +426,7 @@ basic:
|
||||
area: "fonts"
|
||||
allow_any: false
|
||||
heading_font:
|
||||
default: "arial"
|
||||
default: "inter"
|
||||
choices: "BaseFontSetting.values"
|
||||
refresh: true
|
||||
type: list
|
||||
|
@ -0,0 +1,34 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
class SetDefaultFontForExistingSites < ActiveRecord::Migration[7.2]
|
||||
def up
|
||||
return if !Migration::Helpers.existing_site?
|
||||
|
||||
base_font_changed_from_default =
|
||||
DB.query_single("SELECT 1 FROM site_settings WHERE name = 'base_font'").first == 1
|
||||
heading_font_changed_from_default =
|
||||
DB.query_single("SELECT 1 FROM site_settings WHERE name = 'heading_font'").first == 1
|
||||
|
||||
if !base_font_changed_from_default
|
||||
# Type 8 is 'list', arial is the current default font.
|
||||
execute <<~SQL if Migration::Helpers.existing_site?
|
||||
INSERT INTO site_settings(name, data_type, value, created_at, updated_at)
|
||||
VALUES('base_font', 8, 'arial', NOW(), NOW())
|
||||
ON CONFLICT (name) DO NOTHING
|
||||
SQL
|
||||
end
|
||||
|
||||
if !heading_font_changed_from_default
|
||||
# Type 8 is 'list', arial is the current default font.
|
||||
execute <<~SQL if Migration::Helpers.existing_site?
|
||||
INSERT INTO site_settings(name, data_type, value, created_at, updated_at)
|
||||
VALUES('heading_font', 8, 'arial', NOW(), NOW())
|
||||
ON CONFLICT (name) DO NOTHING
|
||||
SQL
|
||||
end
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
@ -2,6 +2,8 @@
|
||||
|
||||
class Wizard
|
||||
class Builder
|
||||
WIZARD_FONTS = %w[lato inter montserrat open_sans poppins roboto]
|
||||
|
||||
def initialize(user)
|
||||
@wizard = Wizard.new(user)
|
||||
end
|
||||
@ -165,28 +167,50 @@ class Wizard
|
||||
themes.add_choice(t[:id], data: { colors: t[:colors] })
|
||||
end
|
||||
|
||||
body_font =
|
||||
step.add_field(
|
||||
id: "body_font",
|
||||
type: "dropdown",
|
||||
value: SiteSetting.base_font,
|
||||
show_in_sidebar: true,
|
||||
)
|
||||
if SiteSetting.base_font != SiteSetting.heading_font
|
||||
body_font =
|
||||
step.add_field(
|
||||
id: "body_font",
|
||||
type: "dropdown",
|
||||
value: SiteSetting.base_font,
|
||||
show_in_sidebar: true,
|
||||
)
|
||||
|
||||
heading_font =
|
||||
step.add_field(
|
||||
id: "heading_font",
|
||||
type: "dropdown",
|
||||
value: SiteSetting.heading_font,
|
||||
show_in_sidebar: true,
|
||||
)
|
||||
heading_font =
|
||||
step.add_field(
|
||||
id: "heading_font",
|
||||
type: "dropdown",
|
||||
value: SiteSetting.heading_font,
|
||||
show_in_sidebar: true,
|
||||
)
|
||||
else
|
||||
site_font =
|
||||
step.add_field(
|
||||
id: "site_font",
|
||||
type: "dropdown",
|
||||
value: SiteSetting.base_font,
|
||||
show_in_sidebar: true,
|
||||
)
|
||||
end
|
||||
|
||||
allowed_fonts = WIZARD_FONTS
|
||||
allowed_fonts << SiteSetting.base_font if !allowed_fonts.include?(SiteSetting.base_font)
|
||||
if !allowed_fonts.include?(SiteSetting.heading_font)
|
||||
allowed_fonts << SiteSetting.heading_font
|
||||
end
|
||||
|
||||
DiscourseFonts
|
||||
.fonts
|
||||
.sort_by { |f| f[:name] }
|
||||
.select do |font|
|
||||
# We only want to display certain fonts in the wizard, others will be accessible
|
||||
# in site settings.
|
||||
allowed_fonts.include?(font[:key])
|
||||
end
|
||||
.sort_by { |font| font[:name] }
|
||||
.each do |font|
|
||||
body_font.add_choice(font[:key], label: font[:name])
|
||||
heading_font.add_choice(font[:key], label: font[:name])
|
||||
body_font&.add_choice(font[:key], label: font[:name])
|
||||
heading_font&.add_choice(font[:key], label: font[:name])
|
||||
site_font&.add_choice(font[:key], label: font[:name])
|
||||
end
|
||||
|
||||
current =
|
||||
@ -217,8 +241,13 @@ class Wizard
|
||||
step.add_field(id: "styling_preview", type: "styling-preview")
|
||||
|
||||
step.on_update do |updater|
|
||||
updater.update_setting(:base_font, updater.fields[:body_font])
|
||||
updater.update_setting(:heading_font, updater.fields[:heading_font])
|
||||
if updater.fields[:site_font].present?
|
||||
updater.update_setting(:base_font, updater.fields[:site_font])
|
||||
updater.update_setting(:heading_font, updater.fields[:site_font])
|
||||
else
|
||||
updater.update_setting(:base_font, updater.fields[:body_font])
|
||||
updater.update_setting(:heading_font, updater.fields[:heading_font])
|
||||
end
|
||||
|
||||
top_menu = SiteSetting.top_menu_map
|
||||
if !updater.fields[:homepage_style].include?("categories") &&
|
||||
|
@ -9,7 +9,7 @@ RSpec.describe Stylesheet::Importer do
|
||||
|
||||
describe "#font" do
|
||||
it "includes font variable" do
|
||||
default_font = ":root{--font-family: Arial, sans-serif}"
|
||||
default_font = ":root{--font-family: Inter, Arial, sans-serif}"
|
||||
expect(compile_css("color_definitions")).to include(default_font)
|
||||
expect(compile_css("embed")).to include(default_font)
|
||||
expect(compile_css("publish")).to include(default_font)
|
||||
|
@ -96,6 +96,15 @@ RSpec.describe Wizard::StepUpdater do
|
||||
expect(SiteSetting.heading_font).to eq("oswald")
|
||||
end
|
||||
|
||||
it "updates both fonts if site_font is used" do
|
||||
updater = wizard.create_updater("styling", site_font: "open_sans", homepage_style: "latest")
|
||||
updater.update
|
||||
expect(updater.success?).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
|
||||
|
||||
context "with colors" do
|
||||
context "with an existing color scheme" do
|
||||
fab!(:color_scheme) { Fabricate(:color_scheme, name: "existing", via_wizard: true) }
|
||||
|
@ -92,12 +92,19 @@ RSpec.describe Wizard::Builder do
|
||||
|
||||
describe "styling" do
|
||||
let(:styling_step) { wizard.steps.find { |s| s.id == "styling" } }
|
||||
let(:font_field) { styling_step.fields.find { |f| f.id == "body_font" } }
|
||||
let(:font_field) { styling_step.fields.find { |f| f.id == "site_font" } }
|
||||
fab!(:theme)
|
||||
let(:colors_field) { styling_step.fields.first }
|
||||
|
||||
before do
|
||||
SiteSetting.remove_override!(:base_font)
|
||||
SiteSetting.remove_override!(:heading_font)
|
||||
end
|
||||
|
||||
it "has the full list of available fonts in alphabetical order" do
|
||||
expect(font_field.choices.map(&:label)).to eq(DiscourseFonts.fonts.map { |f| f[:name] }.sort)
|
||||
expect(font_field.choices.map(&:label)).to eq(
|
||||
["Inter", "Lato", "Montserrat", "Open Sans", "Poppins", "Roboto"],
|
||||
)
|
||||
end
|
||||
|
||||
context "with colors" do
|
||||
|
@ -100,6 +100,13 @@ class PageObjects::Pages::Wizard::StylingStep < PageObjects::Pages::Wizard::Step
|
||||
select_kit.select_row_by_value(palette)
|
||||
end
|
||||
|
||||
def select_font_option(font)
|
||||
select_kit =
|
||||
PageObjects::Components::SelectKit.new(".dropdown-site-font .wizard-container__dropdown")
|
||||
select_kit.expand
|
||||
select_kit.select_row_by_value(font)
|
||||
end
|
||||
|
||||
def select_body_font_option(font)
|
||||
select_kit =
|
||||
PageObjects::Components::SelectKit.new(".dropdown-body-font .wizard-container__dropdown")
|
||||
@ -127,6 +134,12 @@ class PageObjects::Pages::Wizard::StylingStep < PageObjects::Pages::Wizard::Step
|
||||
select_kit.has_selected_value?(palette)
|
||||
end
|
||||
|
||||
def has_selected_font?(font)
|
||||
select_kit =
|
||||
PageObjects::Components::SelectKit.new(".dropdown-site-font .wizard-container__dropdown")
|
||||
select_kit.has_selected_value?(font)
|
||||
end
|
||||
|
||||
def has_selected_body_font?(font)
|
||||
select_kit =
|
||||
PageObjects::Components::SelectKit.new(".dropdown-body-font .wizard-container__dropdown")
|
||||
|
@ -82,13 +82,12 @@ describe "Wizard", type: :system do
|
||||
end
|
||||
|
||||
describe "Wizard Step: Styling" do
|
||||
it "lets user configure styling including fonts and colors" do
|
||||
it "lets user configure styling including font and colors" do
|
||||
wizard_page.go_to_step("styling")
|
||||
expect(wizard_page).to be_on_step("styling")
|
||||
|
||||
wizard_page.styling_step.select_color_palette_option("Dark")
|
||||
wizard_page.styling_step.select_body_font_option("lato")
|
||||
wizard_page.styling_step.select_heading_font_option("merriweather")
|
||||
wizard_page.styling_step.select_font_option("roboto")
|
||||
wizard_page.styling_step.select_homepage_style_option("hot")
|
||||
|
||||
wizard_page.go_to_next_step
|
||||
@ -97,17 +96,32 @@ describe "Wizard", type: :system do
|
||||
expect(Theme.find_default.color_scheme_id).to eq(
|
||||
ColorScheme.find_by(base_scheme_id: "Dark", via_wizard: true).id,
|
||||
)
|
||||
expect(SiteSetting.base_font).to eq("lato")
|
||||
expect(SiteSetting.heading_font).to eq("merriweather")
|
||||
expect(SiteSetting.base_font).to eq("roboto")
|
||||
expect(SiteSetting.heading_font).to eq("roboto")
|
||||
expect(SiteSetting.homepage).to eq("hot")
|
||||
|
||||
wizard_page.go_to_step("styling")
|
||||
|
||||
expect(wizard_page.styling_step).to have_selected_color_palette("Dark")
|
||||
expect(wizard_page.styling_step).to have_selected_body_font("lato")
|
||||
expect(wizard_page.styling_step).to have_selected_heading_font("merriweather")
|
||||
expect(wizard_page.styling_step).to have_selected_font("roboto")
|
||||
expect(wizard_page.styling_step).to have_selected_homepage_style("hot")
|
||||
end
|
||||
|
||||
it "lets user select separate body and heading font if they are already seperate" do
|
||||
SiteSetting.base_font = "poppins"
|
||||
SiteSetting.heading_font = "montserrat"
|
||||
wizard_page.go_to_step("styling")
|
||||
expect(wizard_page).to be_on_step("styling")
|
||||
|
||||
wizard_page.styling_step.select_body_font_option("roboto")
|
||||
wizard_page.styling_step.select_heading_font_option("inter")
|
||||
|
||||
wizard_page.go_to_next_step
|
||||
expect(wizard_page).to be_on_step("ready")
|
||||
|
||||
expect(SiteSetting.base_font).to eq("roboto")
|
||||
expect(SiteSetting.heading_font).to eq("inter")
|
||||
end
|
||||
end
|
||||
|
||||
describe "Wizard Step: Ready" do
|
||||
|
Reference in New Issue
Block a user