Skip to content

Commit

Permalink
FIX: Preload fonts before rerendering wizard style canvas (discourse#…
Browse files Browse the repository at this point in the history
…22361)

]When changing fonts in the `/wizard/steps/styling` step of
the wizard, users would not see the font loaded straight away,
having to switch to another one then back to the original to
see the result. This is because we are using canvas to render
the style preview and this fails with a Chrome-based intervention
when font loading is taking too long:

> [Intervention] Slow network is detected. See
https://www.chromestatus.com/feature/5636954674692096 for more details.
Fallback font will be used while loading:
https://sea2.discourse-cdn.com/business7/fonts/Roboto-Bold.ttf?v=0.0.9

We can get around this by manually loading the fonts selected using
the FontFace JS API when the user selects them and before rerendering
the canvas. This just requires preloading more information about the
fonts if the user is admin so the wizard can query this data.
  • Loading branch information
martin-brennan authored Jul 3, 2023
1 parent 821cf20 commit db80a8c
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Component from "@ember/component";
import PreloadStore from "discourse/lib/preload-store";
import { Promise } from "rsvp";
/*eslint no-bitwise:0 */
import getUrl from "discourse-common/lib/get-url";
Expand Down Expand Up @@ -51,24 +52,78 @@ export default Component.extend({

ctx: null,
loaded: false,
loadingFontVariants: false,

didInsertElement() {
this._super(...arguments);
this.fontMap = PreloadStore.get("fontMap");
this.loadedFonts = new Set();
const c = this.element.querySelector("canvas");
this.ctx = c.getContext("2d");
this.ctx.scale(scale, scale);
this.reload();
},

@observes(
"step.fieldsById.{color_scheme,body_font,heading_font,homepage_style}.value"
)
@observes("step.fieldsById.{color_scheme,homepage_style}.value")
themeChanged() {
this.triggerRepaint();
},

@observes("step.fieldsById.{body_font}.value")
themeBodyFontChanged() {
if (!this.loadingFontVariants) {
this.loadFontVariants(this.wizard.font);
}
},

@observes("step.fieldsById.{heading_font}.value")
themeHeadingFontChanged() {
if (!this.loadingFontVariants) {
this.loadFontVariants(this.wizard.headingFont);
}
},

loadFontVariants(font) {
const fontVariantData = this.fontMap[font.id];

// System font for example does not need to load from a remote source.
if (fontVariantData && !this.loadedFonts.has(font.id)) {
this.loadingFontVariants = true;
const fontFaces = fontVariantData.map((fontVariant) => {
return new FontFace(font.label, `url(${fontVariant.url})`, {
style: "normal",
weight: fontVariant.weight,
});
});

Promise.all(
fontFaces.map((fontFace) =>
fontFace.load().then((loadedFont) => {
document.fonts.add(loadedFont);

// We use our own Set because, though document.fonts.check is available,
// it does not seem very reliable, returning false for fonts that have
// definitely been loaded.
this.loadedFonts.add(font.id);
})
)
)
.then(() => {
this.triggerRepaint();
})
.finally(() => {
this.loadingFontVariants = false;
});
} else if (this.loadedFonts.has(font.id)) {
this.triggerRepaint();
}
},

images() {},

// NOTE: This works for fonts included in a style that is actually using the
// @font-faces on load, but for fonts that we aren't using yet we need to
// make sure they are loaded before rendering the canvas via loadFontVariants.
loadFonts() {
return document.fonts.ready;
},
Expand Down Expand Up @@ -125,8 +180,8 @@ export default Component.extend({
const options = {
ctx,
colors,
font,
headingFont,
font: font?.label,
headingFont: headingFont?.label,
width: this.width,
height: this.height,
};
Expand Down
4 changes: 2 additions & 2 deletions app/assets/javascripts/wizard/addon/models/wizard.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ const Wizard = EmberObject.extend(Evented, {
get font() {
const fontChoice = this.steps.findBy("id", "styling")?.fieldsById
?.body_font;
return fontChoice.choices?.findBy("id", fontChoice.value)?.label;
return fontChoice.choices?.findBy("id", fontChoice.value);
},

get headingFont() {
const fontChoice = this.steps.findBy("id", "styling")?.fieldsById
?.heading_font;
return fontChoice.choices?.findBy("id", fontChoice.value)?.label;
return fontChoice.choices?.findBy("id", fontChoice.value);
},
});

Expand Down
17 changes: 17 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,9 @@ def preload_current_user_data

store_preloaded("topicTrackingStates", MultiJson.dump(hash[:data]))
store_preloaded("topicTrackingStateMeta", MultiJson.dump(hash[:meta]))

# This is used in the wizard so we can preload fonts using the FontMap JS API.
store_preloaded("fontMap", MultiJson.dump(load_font_map)) if current_user.admin?
end

def custom_html_json
Expand Down Expand Up @@ -1065,4 +1068,18 @@ def link_preload
def spa_boot_request?
request.get? && !(request.format && request.format.json?) && !request.xhr?
end

def load_font_map
DiscourseFonts
.fonts
.each_with_object({}) do |font, font_map|
next if !font[:variants]
font_map[font[:key]] = font[:variants].map do |v|
{
url: "#{Discourse.base_url}/fonts/#{v[:filename]}?v=#{DiscourseFonts::VERSION}",
weight: v[:weight],
}
end
end
end
end
92 changes: 92 additions & 0 deletions spec/requests/application_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1133,4 +1133,96 @@ def headers(locale)
end
end
end

describe "preloading data" do
def preloaded_json
JSON.parse(
Nokogiri::HTML5.fragment(response.body).css("div#data-preloaded").first["data-preloaded"],
)
end

context "when user is anon" do
it "preloads the relevant JSON data" do
get "/latest"
expect(response.status).to eq(200)
expect(preloaded_json.keys).to match_array(
[
"site",
"siteSettings",
"customHTML",
"banner",
"customEmoji",
"isReadOnly",
"isStaffWritesOnly",
"activatedThemes",
"#{TopicList.new("latest", Fabricate(:anonymous), []).preload_key}",
],
)
end
end

context "when user is regular user" do
fab!(:user) { Fabricate(:user) }

before { sign_in(user) }

it "preloads the relevant JSON data" do
get "/latest"
expect(response.status).to eq(200)
expect(preloaded_json.keys).to match_array(
[
"site",
"siteSettings",
"customHTML",
"banner",
"customEmoji",
"isReadOnly",
"isStaffWritesOnly",
"activatedThemes",
"#{TopicList.new("latest", Fabricate(:anonymous), []).preload_key}",
"currentUser",
"topicTrackingStates",
"topicTrackingStateMeta",
],
)
end
end

context "when user is admin" do
fab!(:user) { Fabricate(:admin) }

before { sign_in(user) }

it "preloads the relevant JSON data" do
get "/latest"
expect(response.status).to eq(200)
expect(preloaded_json.keys).to match_array(
[
"site",
"siteSettings",
"customHTML",
"banner",
"customEmoji",
"isReadOnly",
"isStaffWritesOnly",
"activatedThemes",
"#{TopicList.new("latest", Fabricate(:anonymous), []).preload_key}",
"currentUser",
"topicTrackingStates",
"topicTrackingStateMeta",
"fontMap",
],
)
end

it "generates a fontMap" do
get "/latest"
expect(response.status).to eq(200)
font_map = JSON.parse(preloaded_json["fontMap"])
expect(font_map.keys).to match_array(
DiscourseFonts.fonts.filter { |f| f[:variants].present? }.map { |f| f[:key] },
)
end
end
end
end

0 comments on commit db80a8c

Please sign in to comment.