Merge pull request #11027 from quarto-dev/feature/brand-yaml-schema-consolidation

Feature/brand yaml schema consolidation

Author: cscheid

src/core/brand/brand.ts

@@ -9,13 +9,15 @@
 import {
   Brand as BrandJson,
   BrandFont,
+  BrandLogoExplicitResource,
   BrandNamedLogo,
   BrandNamedThemeColor,
   BrandStringLightDark,
   BrandTypography,
   BrandTypographyOptionsBase,
   BrandTypographyOptionsHeadings,
 } from "../../resources/types/schema-types.ts";
+import { InternalError } from "../lib/error.ts";
 
 // we can't programmatically convert typescript types to string arrays,
 // so we have to define this manually. They should match `BrandNamedThemeColor` in schema-types.ts
@@ -32,7 +34,6 @@ export const defaultColorNames: BrandNamedThemeColor[] = [
   "danger",
   "light",
   "dark",
-  "emphasis",
   "link",
 ];
 
@@ -42,10 +43,15 @@ const defaultLogoNames: string[] = [
   "large",
 ];
 
+type CanonicalLogoInfo = {
+  light: BrandLogoExplicitResource;
+  dark: BrandLogoExplicitResource;
+};
+
 type ProcessedBrandData = {
   color: Record<string, string>;
   typography: BrandTypography;
-  logo: Record<string, BrandStringLightDark>;
+  logo: Record<string, CanonicalLogoInfo>;
 };
 
 export class Brand {
@@ -86,34 +92,65 @@ export class Brand {
     if (link) {
       typography.link = link;
     }
-    const monospace = this.getFont("monospace");
+    let monospace = this.getFont("monospace");
+    let monospaceInline = this.getFont("monospace-inline");
+    let monospaceBlock = this.getFont("monospace-block");
+
     if (monospace) {
+      if (typeof monospace === "string") {
+        monospace = { family: monospace };
+      }
       typography.monospace = monospace;
     }
-    const monospaceInline = this.getFont("monospace-inline");
+    if (monospaceInline && typeof monospaceInline === "string") {
+      monospaceInline = { family: monospaceInline };
+    }
+    if (monospaceBlock && typeof monospaceBlock === "string") {
+      monospaceBlock = { family: monospaceBlock };
+    }
+
+    // cut off control flow here so the type checker knows these
+    // are not strings
+    if (typeof monospace === "string") {
+      throw new InternalError("should never happen");
+    }
+    if (typeof monospaceInline === "string") {
+      throw new InternalError("should never happen");
+    }
+    if (typeof monospaceBlock === "string") {
+      throw new InternalError("should never happen");
+    }
+
     if (monospace || monospaceInline) {
       typography["monospace-inline"] = {
         ...(monospace ?? {}),
         ...(monospaceInline ?? {}),
       };
     }
-    const monospaceBlock = this.getFont("monospace-block");
+    if (monospaceBlock) {
+      if (typeof monospaceBlock === "string") {
+        monospaceBlock = { family: monospaceBlock };
+      }
+    }
     if (monospace || monospaceBlock) {
       typography["monospace-block"] = {
         ...(monospace ?? {}),
         ...(monospaceBlock ?? {}),
       };
     }
 
-    const logo: Record<string, BrandStringLightDark> = {};
-    for (const logoName of Object.keys(data.logo?.images ?? {})) {
-      logo[logoName] = this.getLogo(logoName);
-    }
-    for (const logoName of Object.keys(data.logo ?? {})) {
-      if (logoName === "images") {
-        continue;
+    const logo: Record<string, CanonicalLogoInfo> = {};
+    for (
+      const size of [
+        "small",
+        "medium",
+        "large",
+      ] as ("small" | "medium" | "large")[]
+    ) {
+      const v = this.getLogo(size);
+      if (v) {
+        logo[size] = v;
       }
-      logo[logoName] = this.getLogo(logoName);
     }
 
     return {
@@ -195,61 +232,42 @@ export class Brand {
     return fonts ?? [];
   }
 
+  getLogoResource(name: string): BrandLogoExplicitResource {
+    const entry = this.data.logo?.images?.[name];
+    if (!entry) {
+      return { path: name };
+    }
+    if (typeof entry === "string") {
+      return { path: entry };
+    }
+    return entry;
+  }
+
   // the same implementation as getColor except we can also return {light,dark}
   // assuming for now that with only contains strings, not {light,dark}
-  getLogo(name: string): BrandStringLightDark {
-    const seenValues = new Set<string>();
-    do {
-      if (seenValues.has(name)) {
-        throw new Error(
-          `Circular reference in _brand.yml color definitions: ${
-            Array.from(seenValues).join(
-              " -> ",
-            )
-          }`,
-        );
-      }
-      seenValues.add(name);
-      if (this.data.logo?.images?.[name]) {
-        name = this.data.logo.images[name] as string;
-      } else if (
-        defaultLogoNames.includes(name as BrandNamedLogo) &&
-        this.data.logo?.[name as BrandNamedLogo]
-      ) {
-        const brandSLD: BrandStringLightDark = this.data
-          .logo[name as BrandNamedLogo]!;
-        if (typeof brandSLD == "string") {
-          name = brandSLD;
-        } else {
-          const ret: BrandStringLightDark = {};
-          // we need to actually-recurse and not just use the loop
-          // because two paths light/dark
-          const light = brandSLD.light;
-          if (light) {
-            const brandSLD2 = this.getLogo(light);
-            if (typeof brandSLD2 == "string") {
-              ret.light = brandSLD2;
-            } else {
-              ret.light = brandSLD2.light;
-            }
-          }
-          const dark = brandSLD.dark;
-          if (dark) {
-            const brandSLD2 = this.getLogo(dark);
-            if (typeof brandSLD2 == "string") {
-              ret.dark = brandSLD2;
-            } else {
-              ret.dark = brandSLD2.light;
-            }
-          }
-          return ret;
-        }
-      } else {
-        return name;
-      }
-    } while (seenValues.size < 100); // 100 ought to be enough for anyone, with apologies to Bill Gates
-    throw new Error(
-      "Recursion depth exceeded 100 in _brand.yml logo definitions",
-    );
+  getLogo(name: "small" | "medium" | "large"): CanonicalLogoInfo | undefined {
+    const entry = this.data.logo?.[name];
+    if (!entry) {
+      return undefined;
+    }
+    if (typeof entry === "string") {
+      const res = this.getLogoResource(entry);
+      return {
+        light: res,
+        dark: res,
+      };
+    }
+    const lightEntry = entry?.light
+      ? this.getLogoResource(entry.light)
+      : undefined;
+    const darkEntry = entry?.dark
+      ? this.getLogoResource(entry.dark)
+      : undefined;
+    if (lightEntry && darkEntry) {
+      return {
+        light: lightEntry,
+        dark: darkEntry,
+      };
+    }
   }
 }

src/core/sass/brand.ts

@@ -285,9 +285,11 @@ const brandTypographyBundle = (
   const resolveHTMLFontInformation = (
     kind: FontKind,
   ): HTMLFontInformation | undefined => {
-    const resolvedFontOptions = brand.data.typography?.[kind];
+    let resolvedFontOptions = brand.data.typography?.[kind];
     if (!resolvedFontOptions) {
       return undefined;
+    } else if (typeof resolvedFontOptions === "string") {
+      resolvedFontOptions = { family: resolvedFontOptions };
     }
     const family = resolvedFontOptions.family;
     const font = getFontFamilies(family);

src/format/reveal/format-reveal.ts

@@ -335,7 +335,7 @@ const determineRevealLogo = (format: Format): string | undefined => {
         return logoInfo;
       } else {
         // what to do about light vs dark?
-        return logoInfo.light ?? logoInfo.dark;
+        return logoInfo?.light.path ?? logoInfo?.dark.path;
       }
     }
   }

src/project/types/website/website-shared.ts

@@ -172,10 +172,9 @@ export async function websiteNavigationConfig(project: ProjectContext) {
       const logo = projectBrand.processedData.logo.medium ??
         projectBrand.processedData.logo.small ??
         projectBrand.processedData.logo.large;
-      if (typeof logo === "string") {
-        sidebars[0].logo = logo;
-      } else if (typeof logo === "object") {
-        sidebars[0].logo = logo.light; // TODO: This needs smarts to work on light+dark themes
+      if (logo) {
+        sidebars[0].logo = logo.light.path; // TODO: This needs smarts to work on light+dark themes
+        sidebars[0]["logo-alt"] = logo.light.alt;
       }
     }
   }
@@ -186,10 +185,9 @@ export async function websiteNavigationConfig(project: ProjectContext) {
     const logo = projectBrand.processedData.logo.small ??
       projectBrand.processedData.logo.medium ??
       projectBrand.processedData.logo.large;
-    if (typeof logo === "string") {
-      navbar.logo = logo;
-    } else if (typeof logo === "object") {
-      navbar.logo = logo.light; // TODO: This needs smarts to work on light+dark themes
+    if (logo) {
+      navbar.logo = logo.light.path; // TODO: This needs smarts to work on light+dark themes
+      navbar["logo-alt"] = logo.light.alt;
     }
   }