Studio: appearance palettes, customization options - #7075
shimmyshimmer wants to merge 2 commits into
Conversation
… controls Adds Standard, Classic, and Minimal color palettes to Appearance settings, each adapting to light and dark mode. Classic is a neutral enterprise look that reserves its blue accent for toggles, badges, and focus rings; Minimal is strictly black, grey, and white. Adds customization options: accent, background, and foreground colors per active mode, UI and code fonts with device font search and font file import, UI and code font sizes, contrast, pointer cursors, reduce motion, font smoothing, and translucent sidebar. Settings persist through the personalization API with backend validation and sync across devices. Restyles dropdowns, buttons, and inputs to match the ChatGPT app in both modes, with fully rounded pills for single-row controls, no drop shadows, and simple straight-line chevrons replacing the rounded arrow icons everywhere.
for more information, see https://pre-commit.ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive appearance customization feature to Unsloth Studio, allowing users to customize colors, fonts, font sizes, contrast, and other interface preferences across standard, classic, and minimal palettes. The backend adds validation and storage for these settings, while the frontend implements state stores, DOM style injection, and settings controls. The review feedback highlights several key areas for improvement: strengthening backend validation for font data URLs to prevent malicious payloads, escaping backslashes in the font sanitization function to avoid CSS syntax breakage, removing redundant sanitization logic in the font name generator, and fixing a UI sync bug in the font size input when values are clamped.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function sanitizeFont(value: unknown): string | null { | ||
| if (typeof value !== "string") return null; | ||
| // Strip characters that could terminate the declaration or smuggle extra | ||
| // CSS through the inline style (the value lands in style.setProperty). | ||
| const cleaned = value | ||
| .replace(/[;{}()<>"']/g, "") | ||
| .trim() | ||
| .slice(0, 200); | ||
| return cleaned.length > 0 ? cleaned : null; | ||
| } |
There was a problem hiding this comment.
The sanitizeFont function strips common CSS-breaking characters but does not strip backslashes (\). If a font name contains a trailing backslash (e.g., MyFont\), it will escape the closing double quote in applyCustomizationToDocument (e.g., "MyFont\", ...), breaking the CSS syntax and preventing subsequent styles from parsing correctly. Strip backslashes as well to ensure robust sanitization.
| function sanitizeFont(value: unknown): string | null { | |
| if (typeof value !== "string") return null; | |
| // Strip characters that could terminate the declaration or smuggle extra | |
| // CSS through the inline style (the value lands in style.setProperty). | |
| const cleaned = value | |
| .replace(/[;{}()<>"']/g, "") | |
| .trim() | |
| .slice(0, 200); | |
| return cleaned.length > 0 ? cleaned : null; | |
| } | |
| function sanitizeFont(value: unknown): string | null { | |
| if (typeof value !== "string") return null; | |
| // Strip characters that could terminate the declaration or smuggle extra | |
| // CSS through the inline style (the value lands in style.setProperty). | |
| const cleaned = value | |
| .replace(/[;{}()\u003c\u003e\\\"'\\\\/]/g, "") | |
| .trim() | |
| .slice(0, 200); | |
| return cleaned.length > 0 ? cleaned : null; | |
| } |
| @field_validator("dataUrl") | ||
| @classmethod | ||
| def _validate_font_data_url(cls, value: str) -> str: | ||
| if not (value.startswith("data:font/") or value.startswith("data:application/")): | ||
| raise ValueError("dataUrl must be a font data URL.") | ||
| return value |
There was a problem hiding this comment.
The current dataUrl validation only checks if the string starts with data:font/ or data:application/. This is extremely loose and allows storing arbitrary or potentially malicious payloads (e.g., data:application/javascript;base64,...) in the database. To prevent storage of garbage or malicious data, use a stricter regex pattern that matches the frontend's FONT_DATA_URL_PATTERN to ensure only valid base64-encoded font data URLs are accepted.
| @field_validator("dataUrl") | |
| @classmethod | |
| def _validate_font_data_url(cls, value: str) -> str: | |
| if not (value.startswith("data:font/") or value.startswith("data:application/")): | |
| raise ValueError("dataUrl must be a font data URL.") | |
| return value | |
| @field_validator("dataUrl") | |
| @classmethod | |
| def _validate_font_data_url(cls, value: str) -> str: | |
| import re | |
| pattern = r"^data:(?:font/(?:woff2?|ttf|otf|sfnt)|application/(?:octet-stream|x-font-\w+|font-\w+));base64,[A-Za-z0-9+/=]+$" | |
| if not re.match(pattern, value): | |
| raise ValueError("dataUrl must be a valid base64-encoded font data URL.") | |
| return value |
| function fontNameFromFile(fileName: string, taken: Set<string>): string { | ||
| const base = | ||
| fileName | ||
| .replace(/\.[^.]+$/, "") | ||
| .replace(/[;{}()<>"']/g, "") | ||
| .replace(/[_-]+/g, " ") | ||
| .trim() | ||
| .slice(0, 60) || "Imported font"; |
There was a problem hiding this comment.
To avoid redundant sanitization in the UI component, we should not duplicate the sanitization logic in fontNameFromFile. Since the font name is already guaranteed to be sanitized on write in the state store, we should reuse the store's sanitization function (sanitizeFont) or use the sanitized name from the store to register the font. This prevents code duplication and ensures the registered font name matches the store name.
function fontNameFromFile(fileName: string, taken: Set<string>): string {
const base =
fileName
.replace(/\.[^.]+$/, "")
.replace(/[_-]+/g, " ")
.trim()
.slice(0, 60) || "Imported font";
return sanitizeFont(base) || "Imported font";
}
References
- Avoid adding redundant sanitization or dead code for values (such as API tokens) that are already guaranteed to be sanitized, validated, or formatted at the source (e.g., on write in the state store).
| const commit = () => { | ||
| const trimmed = draft.trim(); | ||
| if (trimmed === "") { | ||
| onCommit(null); | ||
| return; | ||
| } | ||
| const parsed = Number.parseInt(trimmed, 10); | ||
| if (Number.isNaN(parsed)) { | ||
| setDraft(value === null ? "" : String(value)); | ||
| return; | ||
| } | ||
| onCommit(Math.min(range.max, Math.max(range.min, parsed))); | ||
| }; |
There was a problem hiding this comment.
In FontSizeInput, when a user types an out-of-bounds or decimal value, onCommit is called with the clamped integer value. However, the local draft state is not updated with the clamped value. If the clamped value happens to equal the current store value, the store state won't change, and the useEffect won't trigger to sync the input field. This leaves the out-of-bounds or decimal string visible in the input field. Updating draft with the clamped value inside commit ensures the UI immediately reflects the actual applied value.
const commit = () => {
const trimmed = draft.trim();
if (trimmed === "") {
onCommit(null);
return;
}
const parsed = Number.parseInt(trimmed, 10);
if (Number.isNaN(parsed)) {
setDraft(value === null ? "" : String(value));
return;
}
const clamped = Math.min(range.max, Math.max(range.min, parsed));
setDraft(String(clamped));
onCommit(clamped);
};
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| export function setPalette(next: Palette): void { | ||
| if (typeof window === "undefined") return; | ||
| try { | ||
| window.localStorage.setItem(PALETTE_STORAGE_KEY, next); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c44345d68e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| setVar( | ||
| "--font-sans", | ||
| c.uiFont ? `"${c.uiFont}", ${DEFAULT_SANS_STACK}` : null, | ||
| ); |
There was a problem hiding this comment.
When a user selects a UI font, this only updates --font-sans, but most Studio text is styled through Tailwind's font-sans utility (including html/body via @apply in index.css), and the font theme tokens are declared under @theme inline, so those utilities inline the default font family instead of reading this runtime variable. In that setup the setting changes headings or hand-written font-family: var(--font-sans) rules at best, while the bulk of the UI remains Inter; add a runtime font-family rule/class that consumes this variable or make the Tailwind font utility reference it.
Useful? React with 👍 / 👎.
| function AppearanceCustomizationEffect() { | ||
| const { resolved } = useTheme(); | ||
| const customization = useAppearanceCustomStore((s) => s.customization); | ||
| useEffect(() => { | ||
| applyCustomizationToDocument(customization, resolved); | ||
| }, [customization, resolved]); |
There was a problem hiding this comment.
When the theme is set to System, an OS light/dark change updates the <html> class in theme-store, but useTheme() still snapshots the same string ("system"), so this effect does not re-render and applyCustomizationToDocument is not called with the new resolved mode. Users with different light and dark custom colors will keep the previous mode's inline --background, --foreground, or accent variables until some unrelated customization/theme change happens; include the resolved mode in the subscribed snapshot or otherwise trigger this effect from the media-query change.
Useful? React with 👍 / 👎.
| const REDUCED_MOTION_MAP = { | ||
| system: "user", | ||
| on: "always", | ||
| off: "never", | ||
| } as const; |
There was a problem hiding this comment.
When the OS has prefers-reduced-motion: reduce, selecting the new Off option only maps Motion animations to never; the global CSS @media (prefers-reduced-motion: reduce) rules and components using useReducedMotion() still collapse CSS and local transitions. In that environment the Off setting remains mostly reduced despite the UI saying it is off, so the persisted preference needs to gate or override those system-based reductions too.
Useful? React with 👍 / 👎.
| html.force-reduced-motion *, | ||
| html.force-reduced-motion *::before, | ||
| html.force-reduced-motion *::after { | ||
| animation-duration: 0.01ms !important; | ||
| animation-iteration-count: 1 !important; |
There was a problem hiding this comment.
When a user sets Reduce motion to On, this blanket selector also clamps .animate-spin and generated-image loading dots to a single 0.01ms iteration. The existing OS-level reduced-motion block later keeps those loaders animating because otherwise startup/update/tool progress can look frozen, but this new class path lacks the same exceptions; add matching exemptions for loading indicators under html.force-reduced-motion.
Useful? React with 👍 / 👎.
| export const MAX_IMPORTED_FONTS = 3; | ||
| /** ~1.5 MB file → ~2 MB base64; must stay in sync with the backend cap. */ | ||
| export const MAX_IMPORTED_FONT_DATA_URL_LENGTH = 2_200_000; |
There was a problem hiding this comment.
When a user imports three fonts near the advertised 1.5 MB cap, each one is stored as a base64 data URL in the zustand persist localStorage entry, so the allowed payload can exceed common 5 MiB per-origin quotas before any other Studio data is counted. In that scenario the localStorage write fails and appearance settings stop persisting locally, especially for unauthenticated/offline users; reduce the aggregate cap or store font blobs outside localStorage and persist only references.
Useful? React with 👍 / 👎.
|
Superseded by #7077, which includes additional control and settings changes. |
shimmyshimmer
changed the title
2 participants
What this adds
Appearance settings in Studio previously only offered light/dark. This PR adds color palettes, a full set of appearance customization options, and restyles the core controls (dropdowns, buttons, inputs) for a cleaner, more neutral look in both modes.
Color palettes
Three palettes in Settings > Appearance, each with its own light and dark scheme:
Dark mode surfaces (page, sidebar, cards, popovers, borders) are shared by all three palettes so the app stays consistent; palettes only swap accents. The palette is stored as a
data-paletteattribute on<html>, orthogonal to the existing dark/light class, so the default look is untouched.Customization options
All options apply to the currently active mode only (the other mode keeps its own values), persist through the personalization API, and sync across devices:
The customization applier writes inline CSS variables and gated classes on
<html>, so a default customization leaves the document byte-identical to stock. Persisted payloads are sanitized on every rehydrate so stale local storage can never break the UI.Control restyle
ArrowDown01Icon,ArrowUp01Icon,ArrowUp02Icon,UnfoldMoreIcon) are replaced by simple straight-line chevrons across the app (selects, accordions, navigation, collapsibles, export, data recipes, download manager, folder browser)Backend
PersonalizationAppearancegainspaletteandcustomizationwith strict validation (hex color patterns, font name length, size and contrast ranges, imported font data URL checks).Testing
npm run typecheck,npm run build, andnpm run i18n:checkpass (en, ja, zh-CN, pt-br strings added)test_personalization_settings.pycover palette and customization validation, defaults, imported font limits, and full round-trips