Skip to content

fix: complete Google Font setup in brand panel #2867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 18, 2025
Merged

fix: complete Google Font setup in brand panel #2867

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7abb58
fix: complete Google Font setup in brand panel
drfarrell Sep 18, 2025
eb87f78
fix: auto-refresh webviews after font changes
drfarrell Sep 18, 2025
fa20e69
fix: handle font IDs with special characters in Tailwind config
drfarrell Sep 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions apps/web/client/src/components/store/editor/font/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,14 @@ export class FontManager {
*/
async addFont(font: Font): Promise<boolean> {
try {
const sandbox = this.editorEngine.activeSandbox;
if (!sandbox) {
console.error('No sandbox session found');
return false;
}

await this.ensureConfigFilesExist();

const success = await this.fontConfigManager.addFont(font);
if (success) {
// Update the fonts array
Expand All @@ -165,6 +173,15 @@ export class FontManager {
// Load the new font in the search manager
await this.fontSearchManager.loadFontFromBatch([font]);

// Add font to Tailwind config
await addFontToTailwindConfig(font, sandbox);

// Add font variable to root layout
await this.layoutManager.addFontVariableToRootLayout(font.id);

// Reload all views to apply new font styles
this.editorEngine.frames.reloadAllViews();

Comment on lines +176 to +184
Copy link

@coderabbitai coderabbitai bot Sep 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Handle failures from Tailwind/layout updates and avoid inconsistent state; also coalesce reloads.

addFontToTailwindConfig returns boolean but the result is ignored. If this step fails, you’ll push state, load font, and reload without Tailwind mapping—font won’t render until a later sync (and may never self-heal). Also, this triggers one reload here and likely another from the file watcher.

Suggested fix: fail fast if Tailwind update fails, and switch to a scheduled (coalesced) reload.

Apply this diff in-place:

-                // Add font to Tailwind config
-                await addFontToTailwindConfig(font, sandbox);
+                // Add font to Tailwind config; abort on failure to avoid inconsistent state
+                const tailwindUpdated = await addFontToTailwindConfig(font, sandbox);
+                if (!tailwindUpdated) {
+                    console.error('Failed to update Tailwind config for font:', font.id);
+                    return false;
+                }

-                // Add font variable to root layout
-                await this.layoutManager.addFontVariableToRootLayout(font.id);
+                // Add font variable to root layout (idempotent)
+                await this.layoutManager.addFontVariableToRootLayout(font.id);

-                // Reload all views to apply new font styles
-                this.editorEngine.frames.reloadAllViews();
+                // Coalesce reloads to avoid double refresh with file watcher
+                this.scheduleReload();

Add the helper once in the class (outside the shown range):

// Helper: coalesce multiple reloads in the same tick
private reloadScheduled = false;
private scheduleReload(): void {
  if (this.reloadScheduled) return;
  this.reloadScheduled = true;
  queueMicrotask(() => {
    this.reloadScheduled = false;
    this.editorEngine.frames.reloadAllViews();
  });
}

Is LayoutManager.addFontVariableToRootLayout(id) guaranteed to be idempotent and/or return a boolean? If it returns a boolean, also guard and log on failure.

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "PWD: $(pwd)"
echo "---- rg availability ----"
rg --version || true

echo "Searching for addFontToTailwindConfig..."
rg -n --hidden -S "addFontToTailwindConfig" -g '!*node_modules/*' -C3 || true

echo "Searching for addFontVariableToRootLayout..."
rg -n --hidden -S "addFontVariableToRootLayout" -g '!*node_modules/*' -C3 || true

echo "Searching for addFontVariable (broader)..."
rg -n --hidden -S "addFontVariable" -g '!*node_modules/*' -C3 || true

echo "Searching for LayoutManager class or interface..."
rg -n --hidden -S "class .*LayoutManager|interface .*LayoutManager" -g '!*node_modules/*' -C3 || true

echo "Searching for scheduleReload/reloadScheduled/scheduleReload"
rg -n --hidden -S "scheduleReload|reloadScheduled|reloadScheduled" -g '!*node_modules/*' -C3 || true

echo "Printing apps/web/client/src/components/store/editor/font/index.ts (if present)"
if [ -f "apps/web/client/src/components/store/editor/font/index.ts" ]; then
  sed -n '1,320p' apps/web/client/src/components/store/editor/font/index.ts
else
  echo "File not found: apps/web/client/src/components/store/editor/font/index.ts"
fi

# If any files define addFontToTailwindConfig or addFontVariableToRootLayout, print them
files=$(rg -n --hidden -S -l "addFontToTailwindConfig" -g '!*node_modules/*' || true)
if [ -n "$files" ]; then
  for f in $files; do
    echo "==== FOUND addFontToTailwindConfig IN: $f ===="
    sed -n '1,240p' "$f"
  done
else
  echo "No files define addFontToTailwindConfig"
fi

files2=$(rg -n --hidden -S -l "addFontVariableToRootLayout" -g '!*node_modules/*' || true)
if [ -n "$files2" ]; then
  for f in $files2; do
    echo "==== FOUND addFontVariableToRootLayout IN: $f ===="
    sed -n '1,240p' "$f"
  done
else
  echo "No files define addFontVariableToRootLayout"
fi

# Also print any LayoutManager file matches
lm=$(rg -n --hidden -S -g '!*node_modules/*' "class .*LayoutManager|interface .*LayoutManager" || true)
if [ -n "$lm" ]; then
  echo "==== LayoutManager matches ===="
  echo "$lm"
  echo ""
  # print the files referenced
  for f in $(echo "$lm" | awk -F: '{print $1}' | sort -u); do
    echo "---- $f ----"
    sed -n '1,240p' "$f"
  done
else
  echo "No obvious LayoutManager class/interface matches"
fi

Length of output: 11920

🏁 Script executed:

#!/bin/bash
set -euo pipefail
cd /home/jailuser/git || exit 1

DIR="apps/web/client/src/components/store/editor/font"
echo "PWD: $(pwd)"
echo "Checking directory: $DIR"
if [ -d "$DIR" ]; then
  ls -la "$DIR"
else
  echo "Directory not found: $DIR"
fi

for fname in tailwind-config.ts tailwind-config.tsx layout-manager.ts layout-manager.tsx layout-manager.js; do
  fpath="$DIR/$fname"
  echo
  echo "---- $fpath ----"
  if [ -f "$fpath" ]; then
    sed -n '1,240p' "$fpath"
  else
    echo "File not found: $fpath"
  fi
done

echo
echo "Searching repository for addFontToTailwindConfig (avoid rg filters)"
find . -type f \( -name '*.ts' -o -name '*.tsx' -o -name '*.js' -o -name '*.jsx' \) -not -path './node_modules/*' -print0 | xargs -0 grep -n --line-number --color=never "addFontToTailwindConfig" || true

echo
echo "Searching repository for addFontVariableToRootLayout"
find . -type f \( -name '*.ts' -o -name '*.tsx' -o -name '*.js' -o -name '*.jsx' \) -not -path './node_modules/*' -print0 | xargs -0 grep -n --line-number --color=never "addFontVariableToRootLayout" || true

echo
echo "Searching for LayoutManager class or file references"
find . -type f \( -name '*.ts' -o -name '*.tsx' -o -name '*.js' \) -not -path './node_modules/*' -print0 | xargs -0 grep -n --line-number --color=never -E "class +LayoutManager|interface +LayoutManager|export +class +LayoutManager" || true

Length of output: 14016

Fail fast on Tailwind update, guard layout update, and coalesce reloads.

addFontToTailwindConfig(...) returns Promise and can fail; LayoutManager.addFontVariableToRootLayout(...) returns Promise (idempotent) but its result is ignored. Abort on Tailwind failure, guard/log layout failure, and replace immediate reload with a coalesced reload to avoid duplicate refreshes (also update the other call at index.ts:402).

Apply this diff in-place:

-                // Add font to Tailwind config
-                await addFontToTailwindConfig(font, sandbox);
-
-                // Add font variable to root layout
-                await this.layoutManager.addFontVariableToRootLayout(font.id);
-
-                // Reload all views to apply new font styles
-                this.editorEngine.frames.reloadAllViews();
+                // Add font to Tailwind config; abort on failure to avoid inconsistent state
+                const tailwindUpdated = await addFontToTailwindConfig(font, sandbox);
+                if (!tailwindUpdated) {
+                    console.error('Failed to update Tailwind config for font:', font.id);
+                    return false;
+                }
+
+                // Add font variable to root layout (idempotent) and guard failure
+                const layoutUpdated = await this.layoutManager.addFontVariableToRootLayout(font.id);
+                if (!layoutUpdated) {
+                    console.error('Failed to add font variable to root layout for font:', font.id);
+                    return false;
+                }
+
+                // Coalesce reloads to avoid double refresh with file watcher
+                this.scheduleReload();

Add the helper once in the class (outside the shown range):

// Helper: coalesce multiple reloads in the same tick
private reloadScheduled = false;
private scheduleReload(): void {
  if (this.reloadScheduled) return;
  this.reloadScheduled = true;
  queueMicrotask(() => {
    this.reloadScheduled = false;
    this.editorEngine.frames.reloadAllViews();
  });
}

File: apps/web/client/src/components/store/editor/font/index.ts (occurrences at ~lines 176–184 and ~402).

🤖 Prompt for AI Agents 
In apps/web/client/src/components/store/editor/font/index.ts around lines
176–184 (and also update the other occurrence near line 402), the code currently
awaits addFontToTailwindConfig(...), calls addFontVariableToRootLayout(...) but
ignores its result, and calls editorEngine.frames.reloadAllViews() immediately;
change this so that you first check the boolean result of
addFontToTailwindConfig(...) and abort/throw/return if it fails, then call
LayoutManager.addFontVariableToRootLayout(...) and log a warning if it returns
false (do not throw), and replace the direct reloadAllViews() call with a call
to a class-level coalescing helper (add the provided reloadScheduled and
scheduleReload() helper once in the class outside the shown range) so multiple
reload requests in the same tick are coalesced; also update the other
reloadAllViews() call at ~line 402 to use scheduleReload() instead of calling
reloadAllViews() directly.

return true;
}
return false;
Expand Down Expand Up @@ -195,6 +212,9 @@ export class FontManager {
this._defaultFont = null;
}

// Reload all views to apply font removal
this.editorEngine.frames.reloadAllViews();

return result;
}
return false;
Expand Down Expand Up @@ -388,6 +408,9 @@ export class FontManager {
this._fonts = currentFonts;
// Update font search manager with current fonts
this.fontSearchManager.updateFontsList(this._fonts);

// Reload all views to apply font changes
this.editorEngine.frames.reloadAllViews();
}

this.previousFonts = currentFonts;
Expand Down