Skip to content

refactor: upload font and set default font#2386

Merged
Kitenite merged 23 commits into
mainfrom
refactor/update-font
Jul 17, 2025
Merged

refactor: upload font and set default font#2386
Kitenite merged 23 commits into
mainfrom
refactor/update-font

Conversation

@spartan-vutrannguyen

@spartan-vutrannguyen spartan-vutrannguyen commented Jul 8, 2025

Copy link
Copy Markdown
Contributor

Description

Related Issues

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Release
  • Refactor
  • Other (please describe):

Testing

Screenshots (if applicable)

  1. Upload custom fonts
Screen.Recording.2025-07-15.at.17.39.52.mp4
  1. Remove fonts
Screen.Recording.2025-07-15.at.17.32.33.mp4
  1. Set default
Screen.Recording.2025-07-15.at.17.31.55.mov

Additional Notes


Important

Refactor font management to support custom font uploads and default font settings, integrating with Tailwind CSS and enhancing AST manipulation utilities.

  • Behavior:
    • Refactor font management to support uploading custom fonts and setting default fonts.
    • Introduces FontManager, FontUploadManager, FontSearchManager, and LayoutManager for handling font operations.
    • Updates font configuration in font-config-manager.ts and integrates with Tailwind CSS in tailwind-config.ts.
  • Utilities:
    • Adds utility functions in ast-generators.ts, ast-manipulators.ts, and font-extractors.ts for AST manipulation and font extraction.
    • Introduces convertRawFont() and cleanComma() in utils.ts.
  • Models:
    • Adds Font, RawFont, FontConfig, and FontUploadFile interfaces in models/src/font/index.ts.
    • Defines RouterType and RouterConfig in models/src/next/index.ts.
  • Misc:
    • Updates constants.ts to change FONT_FOLDER and FONT_CONFIG paths.
    • Refactors sandbox/index.ts to detect router type and manage file indexing.

This description was created by Ellipsis for 9f3a44f. You can customize this summary. It will automatically update as commits are pushed.

@vercel

vercel Bot commented Jul 8, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 17, 2025 7:28pm
web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 17, 2025 7:28pm

@supabase

supabase Bot commented Jul 8, 2025

Copy link
Copy Markdown

This pull request has been ignored for the connected project wowaemfasoptxrdjhilu because there are no changes detected in apps/backend/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@spartan-vutrannguyen
spartan-vutrannguyen marked this pull request as draft July 8, 2025 22:19
Comment thread packages/fonts/src/helper.ts Outdated
Comment thread apps/web/client/src/components/store/editor/font/index.ts Outdated
Comment thread apps/web/client/src/components/store/editor/font/index.ts Outdated
Comment thread packages/fonts/src/utils.ts Outdated
@Kitenite

Kitenite commented Jul 9, 2025

Copy link
Copy Markdown
Contributor

Could you fix merge issues and address the Graphite comments? Thanks

/**
* Sets up file watcher for the font config file
*/
private setupFontConfigFileWatcher(): void {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't understand the need for this watcher. We should actually remove the watcher and event bus entirely and listen for editorEngine.sandbox.files changing in reaction or in observer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, I wrote this earlier while testing with memfs. It works independently of the cache system, so we can ensure it always has the latest update from the server.
But maybe we dont need it now.

this.fontConfigFileWatcher = sandbox.fileEventBus.subscribe('*', async (event) => {
const normalizedFontConfigPath = normalizePath(this.fontConfigPath);
const affectsFont = event.paths.some(
(path) => normalizePath(path) === normalizedFontConfigPath,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this checking for if the path is exactly the config path? Not if the font files changes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This checks whether the font config file has changed.
The event.paths array contains the list of files that were modified.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok thanks for clarifying

setIsLoading(false);
}
};
console.log('fontManager.defaultFont', fontManager.defaultFont);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove or disable debug logging (console.log) before production release.

Suggested change
console.log('fontManager.defaultFont', fontManager.defaultFont);

Comment thread apps/web/client/src/components/store/editor/font/FontConfigManager.ts Outdated
@@ -0,0 +1,300 @@
import type { ParseResult } from '@babel/parser';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please name these with the same format we've been using

export const parseRepoUrl = (repoUrl: string): { owner: string; repo: string } => {
const match = repoUrl.match(/github\.com\/([^/]+)\/([^/]+)(?:\.git)?/);
if (!match || !match[1] || !match[2]) {
const match = /github\.com\/([^/]+)\/([^/]+)(?:\.git)?/.exec(repoUrl);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good idea to test this.

@@ -0,0 +1,143 @@
# Font Management

This directory contains a modular, maintainable system for managing fonts in the Onlook editor. The original monolithic `FontManager` has been refactored into focused, testable classes, each handling a specific concern of font management.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would remove this. Documents like these have a very short lifetime and the code itself should be self-documenting

</TooltipPortal>
</Tooltip>

<span

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removed Tooltip wrapper. Consider adding a title attribute on the span for accessibility if the font name is truncated.

@Kitenite
Kitenite merged commit 4d250e5 into main Jul 17, 2025
3 of 5 checks passed
@Kitenite
Kitenite deleted the refactor/update-font branch July 17, 2025 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants