mirror of
https://github.com/discourse/discourse.git
synced 2024-11-29 04:03:57 -06:00
FIX: Preload fonts before rerendering wizard style canvas (#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.
This commit is contained in:
parent
821cf201f5
commit
db80a8ce79
@ -1,4 +1,5 @@
|
|||||||
import Component from "@ember/component";
|
import Component from "@ember/component";
|
||||||
|
import PreloadStore from "discourse/lib/preload-store";
|
||||||
import { Promise } from "rsvp";
|
import { Promise } from "rsvp";
|
||||||
/*eslint no-bitwise:0 */
|
/*eslint no-bitwise:0 */
|
||||||
import getUrl from "discourse-common/lib/get-url";
|
import getUrl from "discourse-common/lib/get-url";
|
||||||
@ -51,24 +52,78 @@ export default Component.extend({
|
|||||||
|
|
||||||
ctx: null,
|
ctx: null,
|
||||||
loaded: false,
|
loaded: false,
|
||||||
|
loadingFontVariants: false,
|
||||||
|
|
||||||
didInsertElement() {
|
didInsertElement() {
|
||||||
this._super(...arguments);
|
this._super(...arguments);
|
||||||
|
this.fontMap = PreloadStore.get("fontMap");
|
||||||
|
this.loadedFonts = new Set();
|
||||||
const c = this.element.querySelector("canvas");
|
const c = this.element.querySelector("canvas");
|
||||||
this.ctx = c.getContext("2d");
|
this.ctx = c.getContext("2d");
|
||||||
this.ctx.scale(scale, scale);
|
this.ctx.scale(scale, scale);
|
||||||
this.reload();
|
this.reload();
|
||||||
},
|
},
|
||||||
|
|
||||||
@observes(
|
@observes("step.fieldsById.{color_scheme,homepage_style}.value")
|
||||||
"step.fieldsById.{color_scheme,body_font,heading_font,homepage_style}.value"
|
|
||||||
)
|
|
||||||
themeChanged() {
|
themeChanged() {
|
||||||
this.triggerRepaint();
|
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() {},
|
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() {
|
loadFonts() {
|
||||||
return document.fonts.ready;
|
return document.fonts.ready;
|
||||||
},
|
},
|
||||||
@ -125,8 +180,8 @@ export default Component.extend({
|
|||||||
const options = {
|
const options = {
|
||||||
ctx,
|
ctx,
|
||||||
colors,
|
colors,
|
||||||
font,
|
font: font?.label,
|
||||||
headingFont,
|
headingFont: headingFont?.label,
|
||||||
width: this.width,
|
width: this.width,
|
||||||
height: this.height,
|
height: this.height,
|
||||||
};
|
};
|
||||||
|
@ -41,13 +41,13 @@ const Wizard = EmberObject.extend(Evented, {
|
|||||||
get font() {
|
get font() {
|
||||||
const fontChoice = this.steps.findBy("id", "styling")?.fieldsById
|
const fontChoice = this.steps.findBy("id", "styling")?.fieldsById
|
||||||
?.body_font;
|
?.body_font;
|
||||||
return fontChoice.choices?.findBy("id", fontChoice.value)?.label;
|
return fontChoice.choices?.findBy("id", fontChoice.value);
|
||||||
},
|
},
|
||||||
|
|
||||||
get headingFont() {
|
get headingFont() {
|
||||||
const fontChoice = this.steps.findBy("id", "styling")?.fieldsById
|
const fontChoice = this.steps.findBy("id", "styling")?.fieldsById
|
||||||
?.heading_font;
|
?.heading_font;
|
||||||
return fontChoice.choices?.findBy("id", fontChoice.value)?.label;
|
return fontChoice.choices?.findBy("id", fontChoice.value);
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -661,6 +661,9 @@ class ApplicationController < ActionController::Base
|
|||||||
|
|
||||||
store_preloaded("topicTrackingStates", MultiJson.dump(hash[:data]))
|
store_preloaded("topicTrackingStates", MultiJson.dump(hash[:data]))
|
||||||
store_preloaded("topicTrackingStateMeta", MultiJson.dump(hash[:meta]))
|
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
|
end
|
||||||
|
|
||||||
def custom_html_json
|
def custom_html_json
|
||||||
@ -1065,4 +1068,18 @@ class ApplicationController < ActionController::Base
|
|||||||
def spa_boot_request?
|
def spa_boot_request?
|
||||||
request.get? && !(request.format && request.format.json?) && !request.xhr?
|
request.get? && !(request.format && request.format.json?) && !request.xhr?
|
||||||
end
|
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
|
end
|
||||||
|
@ -1133,4 +1133,96 @@ RSpec.describe ApplicationController do
|
|||||||
end
|
end
|
||||||
end
|
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
|
end
|
||||||
|
Loading…
Reference in New Issue
Block a user