Unsloth: appearance palettes, customization options, and control restyle - #7077
Conversation
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 scoped to the active mode: accent, background, and foreground colors with an in-app color picker, UI and code fonts with a searchable dropdown covering bundled, device, and imported fonts, 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 core controls for a cleaner, flatter look in both modes: bordered white input fields, fully rounded pills for single-row controls, no drop shadows, simple straight-line chevrons replacing all rounded arrow icons, and consistent hover tones in dropdown menus. Popovers now portal into the open dialog so their lists scroll correctly inside modal dialogs. Moves Language into General settings and Chat defaults into the Chat tab above the Canvas section.
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 comprehensive appearance customization settings to Unsloth Studio, allowing users to configure custom color palettes, UI and code fonts (including importing custom font files), font sizes, contrast, pointer cursors, reduced motion, and translucent sidebars. These settings are fully integrated with backend Pydantic models, synchronized with the user profile, and localized across multiple languages. Additionally, various UI components have been updated to use standard chevron icons instead of generic arrow icons. The review feedback suggests strengthening the backend validation for imported fonts to prevent potential CSS injection and ensure that only valid base64-encoded font data URLs are stored.
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.
| class PersonalizationImportedFont(BaseModel): | ||
| model_config = ConfigDict(extra = "ignore") | ||
|
|
||
| name: str = Field(..., min_length = 1, max_length = 100) | ||
| dataUrl: str = Field(..., max_length = MAX_FONT_DATA_URL_LENGTH) | ||
|
|
||
| @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 backend validation for PersonalizationImportedFont can be strengthened to prevent potential security and correctness issues. Currently, name is not validated against invalid CSS characters, which could lead to CSS injection if bypassed. Additionally, dataUrl only checks prefixes, allowing arbitrary non-font data to be stored.
We should add a validator for name to reject characters like ;{}()<>'" and use a strict regex pattern for dataUrl that matches the frontend's FONT_DATA_URL_PATTERN exactly.
class PersonalizationImportedFont(BaseModel):
model_config = ConfigDict(extra = "ignore")
name: str = Field(..., min_length = 1, max_length = 100)
dataUrl: str = Field(..., max_length = MAX_FONT_DATA_URL_LENGTH)
@field_validator("name")
@classmethod
def _validate_font_name(cls, value: str) -> str:
if any(c in value for c in ";{}()<>'\""):
raise ValueError("Font name contains invalid characters.")
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
There was a problem hiding this comment.
This is already covered at head. _check_font_name rejects the CSS/injection characters plus control chars below 0x20 for both imported and selected font names, and _FONT_DATA_URL_PATTERN is applied with fullmatch so a trailing newline is rejected too. The model also uses extra='ignore', so unknown keys are dropped.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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: ae50aa7987
ℹ️ 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".
| 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.
With MAX_IMPORTED_FONTS set to 3 while each data URL may be 2.2 MB, the zustand persist store can attempt to write more than 6 MB of font data into localStorage. In browsers/WebViews with a ~5 MiB per-origin quota, importing the allowed third ~1.5 MB font will throw or fail persistence before personalization sync, so the UI advertises a supported state that users cannot reliably save; consider an aggregate cap below the storage quota or storing font blobs outside localStorage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
There is already an aggregate cap. MAX_TOTAL_IMPORTED_FONT_DATA_URL_LENGTH (4.4M) is enforced in sanitizeImportedFonts and mirrored server-side, so the case where a third font pushes the total past the quota cannot happen.
| } | ||
|
|
||
| el.classList.toggle("pointer-cursors", c.pointerCursors); | ||
| el.classList.toggle("force-reduced-motion", c.reduceMotion === "on"); |
There was a problem hiding this comment.
When the OS has prefers-reduced-motion: reduce enabled and the user selects the new Off option, this leaves the document with no override class distinct from system, so the global @media (prefers-reduced-motion: reduce) rules in index.css still collapse CSS/Tailwind/Radix animations. MotionConfig only fixes JS-driven motion; add an explicit "force motion" hook or gate those media rules so off actually restores CSS animations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Off already opts out of the OS preference. applyCustomizationToDocument adds html.force-motion when reduceMotion is off, and every prefers-reduced-motion block in index.css is scoped to html:not(.force-motion), so CSS/Radix animations re-enable.
Neutralizes focus and selection rings across all palettes so highlighted elements, including typing boxes and the selected palette card, never take the accent color. The custom accent no longer recolors rings. Restyles the color controls as filled pills showing the hex value inside, with text and border contrast picked from the color's luminance. Menus in popovers now match the app's dropdown menus: rounded-lg corners, tighter padding, accent hover rows, and a bordered search field. Popovers inside modal dialogs are modal so their lists scroll with the wheel. Outline buttons share the same dark fills as dropdown triggers. Adds heading and chat font options next to the UI and code fonts, each using the searchable font dropdown and persisting through the personalization API. Removes the translucent sidebar option end to end. Adds settings search: a search field at the top of the settings sidebar that filters setting names across every tab, grouped by tab with icons, and jumps to the tab on click.
The settings dialog nav, its close button, the model selector, and the project switcher hovered with hardcoded blue tinted greys (#3a3d43, #2d2e32) in dark mode while every menu and sidebar uses --accent. All hover and active pill fills now use the accent token so dark hovers are the same everywhere and adapt to the active palette.
Widens the settings dialog to 880px and the sidebar column to 248px so the search field has more room. The search pill aligns with the left start of the Settings title, gets more spacing above and below, and its icon and placeholder sit slightly further left. Search results now jump to the exact setting: rows and sections expose their label as a data attribute, and picking a result opens the tab, scrolls the matched row into view, and flashes it briefly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d92b56448
ℹ️ 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".
| 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 the new Reduce motion setting is On, this selector matches every descendant, including .animate-spin and .generated-image-loading-dot, and its !important duration/iteration rules override the existing reduced-motion exceptions later in this file that keep loaders animated because freezing them removes progress feedback. In any loading/export/update state with the user setting forced on, spinners become static, so please carry over the loader exceptions for the force-reduced-motion path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The loader exceptions are already carried into the force path. After the blanket rule, html.force-reduced-motion .animate-spin, .generated-image-loading-dot and .loading-bar-slide re-assert animation-iteration-count: infinite, and the more specific selector wins among competing !important rules, so spinners keep spinning.
| return ( | ||
| <PopoverPrimitive.Portal> | ||
| <PopoverPrimitive.Portal | ||
| container={container ?? dialogContainer ?? undefined} |
There was a problem hiding this comment.
In SettingsDialog no container prop is passed to DialogContent, so DialogPortalContainerContext is null (dialog.tsx initializes it from the optional container rather than the content element). This fallback still body-portals the new Appearance font popovers; inside a modal dialog Radix's body scroll lock swallows wheel events on body-portaled content, so long font lists cannot be mouse-wheel scrolled. Provide the dialog content element as the context value or pass a real container.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The flagged font popovers are modal, and Radix wraps modal popover content in its own RemoveScroll. react-remove-scroll only lets the topmost lock handle wheel events, so the dialog lock is silenced while the popover is open and its own list (overflow-y-auto max-h-72) scrolls. The null portal container is harmless for these modal popovers.
| 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 setting is system, the matchMedia callback in the theme store updates the dark/light class directly, but useTheme() still snapshots only the string "system", so React does not re-render this effect when the OS color scheme changes. With per-mode custom colors set, the inline --background/--foreground variables from the previous mode remain and override the newly applied class until another settings change or reload; subscribe to the resolved mode itself or have the media callback re-run the customization applier.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
useTheme already snapshots the resolved mode through a second useSyncExternalStore, and the matchMedia change handler calls the React notify, so AppearanceCustomizationEffect (keyed on [customization, resolved]) re-runs and re-applies the per-mode colors on an OS light/dark flip under system.
The search field now starts and ends at the same edges as the nav hover pills instead of being inset to the title text.
Reduce motion Off now opts back out of the OS reduced-motion preference for CSS animations via a force-motion class that the media rules skip, and forcing reduce motion On keeps the loader exceptions (spinners, loading dots, progress bars) animating. When the color scheme follows the system, the resolved mode is now part of the theme store snapshot, so an OS scheme flip re-renders consumers and reapplies per-mode custom colors instead of leaving stale inline variables from the previous mode. Imported fonts get an aggregate size cap (4.4M characters) on both the frontend sanitizer and the backend model so the persisted store always fits browser localStorage quotas, with a clear error toast when an import would exceed it. Backend validation also tightens imported font names (rejects CSS delimiter characters) and requires strict base64 font data URLs, matching the frontend patterns.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b417b4c9dd
ℹ️ 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".
| } | ||
| } | ||
| for (const [name, dataUrl] of wanted) { | ||
| if (registeredFontFaces.has(name)) continue; |
There was a problem hiding this comment.
When personalization sync or persisted state replaces an imported font with the same sanitized family name but a different dataUrl, this early continue leaves the old FontFace registered in document.fonts. The store now contains the new bytes, but any UI using that family keeps rendering the previous font until a reload or removal; track the registered data URL and delete/re-add the face when it changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. document.fonts is a set of FontFace objects, not keyed by family, so a same-name re-import kept the stale face. Now tracking the dataUrl and deleting then re-adding when the bytes change. Fixed in bd1b9c7.
Adds a Show greeting sloth switch to Settings > Profile. The chat welcome hides the mascot when it is off. The preference persists locally and through the personalization API, with backend validation and tests, and the row is reachable from settings search in all four locales.
Settings sidebar puts search on top with the tab list under a small Settings label. Combobox popups scroll with the wheel inside dialogs by falling back to manual list scrolling while a dialog scroll lock is active, and the local model selector popover became modal for the same reason. Number inputs swap native spinners for a shared grey stepper that clamps to min, max, and step. Run settings fields in light mode use the same white fill and border as the settings dialog. Selection and focus rings derive from each palette's border color instead of near black, hover borders soften the same way, the Classic sidebar stays white like Standard, decorative greens follow the palette accent, and meaning-carrying marks like the hub verified badge keep the brand green in every palette.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3319fca079
ℹ️ 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".
| className={cn( | ||
| "group/number relative inline-flex items-center", | ||
| /(?:^|\s)w-full(?:\s|$)/.test(className ?? "") ? "w-full" : "w-fit", | ||
| )} |
There was a problem hiding this comment.
For type="number" the component now wraps the input in an inline-flex span, but the wrapper defaults to w-fit unless the caller explicitly passes w-full. The shared Input previously always contributed w-full, and many existing numeric callers rely on that default without specifying a width, so those fields now shrink to their intrinsic width instead of filling their grid/row; make the wrapper preserve the default full-width behavior or derive width from the base classes as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right, the wrapper defaulted to w-fit and collapsed fields that used to be full width in flex/grid cells. The wrapper now defaults to w-full min-w-0 and mirrors any explicit w-/max-w from the caller so the stepper stays pinned to the field edge. Fixed in bd1b9c7.
| resetAll: () => set({ customization: DEFAULT_CUSTOMIZATION }), | ||
| }), | ||
| { | ||
| name: "unsloth_appearance_customization", |
There was a problem hiding this comment.
This adds a new persisted localStorage key for appearance customization, but resetAllPrefs() only removes the old appearance keys (theme and locale) and never removes this store or the new palette key. After changing custom colors/fonts/imported fonts or the palette, using “Reset all local preferences” and reloading will leave those appearance overrides active, which defeats the reset flow especially if a bad customization is what the user is trying to clear.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, reset skipped the palette and appearance customization keys. Added both to PREFS_KEYS so a reload rehydrates to stock. Fixed in bd1b9c7.
Switching palettes restyles the whole page the moment data-palette lands on the html element, but the React re-render that moves the selection classes arrives later, so the ring and check briefly stayed on the previous card with the new palette's colors. The active ring and check now key off html[data-palette] in CSS, so they swap in the same style pass that swaps the tokens. Also adds breathing room around the settings search bar and under the Settings label, shortens the greeting sloth description, and renames the avatar section to Or pick a sloth profile picture in all locales.
655369d to
1f5ef35
Compare
shimmyshimmer
changed the title
Puts the ring tokens back to their fixed per palette values and removes the hover border darkening, undoing the derived border experiment. The selected palette card no longer shows a check since the ring already marks it. The settings sidebar search bar, nav pills, and search results get a little side padding, and the Settings label lines up with the pill text.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f5ef3520c
ℹ️ 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".
| "--font-mono", | ||
| c.codeFont ? `"${c.codeFont}", ${DEFAULT_MONO_STACK}` : null, | ||
| ); |
There was a problem hiding this comment.
When a user selects a custom Code font, this only updates --font-mono, but the main chat code surfaces still bypass that token: index.css hard-codes .aui-thread-root [data-streamdown="code-block"] pre/code and inline code to "Fira Code", ui-monospace, monospace. In that common chat/code-review path, the new setting appears saved but leaves code fences and inline code unchanged; route those rules through the same custom mono token so the setting actually applies.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, the chat code fences and inline code hard-coded Fira Code and never read the Code font. Added a --custom-code-font token that those rules fall back through, so an unset value stays Fira Code. Fixed in bd1b9c7.
| /> | ||
| </TooltipProvider> | ||
| <ThemeProvider attribute="class" defaultTheme="system" enableSystem={true}> | ||
| <MotionConfig reducedMotion={REDUCED_MOTION_MAP[reduceMotion]}> |
There was a problem hiding this comment.
When a user sets Reduce motion to On on a system whose OS preference is not reduced, this only feeds Motion components through MotionConfig; it does not affect the direct canvas-confetti celebrations used by onboarding and the guided tour, which either only check matchMedia or do not check reduced motion at all. Completing those flows can still launch full-screen confetti despite the explicit app setting, so pass this setting into those helpers or suppress celebrations when it is forced on.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right, confetti only checked the OS preference, not the in-app Reduce motion. Both the onboarding and tour celebrations now gate on the resolved setting (on/off wins, else OS). Fixed in bd1b9c7.
- Derive focus and selection rings from the border color so indicators stay 1px and adapt to every theme and palette - Suppress mouse focus rings except on pressed controls to remove the selection flash on the avatar and palette pickers - Defer settings panel rendering so the active nav pill updates instantly - Customizable sidebar user menu with drag to reorder and shortcuts to the settings tabs - Grey hover for the standard light palette instead of green - Borderless controls in dark mode with fill based focus states - Profile picture: no picture option, pencil edit icon, atomic selection - Font dropdowns: narrower triggers and the resolved default shown as Inter Variable (Default) - System prompt border darkens on focus - New appearance setting to swap edge fades for thin divider lines - Move the theme bootstrap to an external script to satisfy CSP
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f756440e8d
ℹ️ 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".
| Math.max(left, window.innerWidth - left), | ||
| Math.max(top, window.innerHeight - top) | ||
| ) | ||
| document.documentElement.animate( |
There was a problem hiding this comment.
When Appearance → Reduce motion is set to On, the new MotionConfig and force-reduced-motion CSS rules do not affect the View Transition API/Web Animations used here. In browsers with startViewTransition, toggling dark/light mode from the account menu still runs the circular clip-path animation despite the explicit app setting; skip the view transition or force a zero-duration path when reduced motion is forced on.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, the circular reveal runs via element.animate() targeting ::view-transition-new(root), which the Web Animations API drives independently of CSS, so force-reduced-motion could not reach it. Now skipping the view transition and applying the theme instantly when reduced motion is set. Fixed in bd1b9c7.
| setVar( | ||
| "--font-sans", | ||
| c.uiFont ? `"${c.uiFont}", ${DEFAULT_SANS_STACK}` : null, | ||
| ); |
There was a problem hiding this comment.
When a user selects a custom UI font, this only updates --font-sans, but the base body/html font still comes from Tailwind's font-sans utility defined under the hard-coded @theme inline --font-sans in index.css. As a result, text that just inherits the app font keeps rendering Inter while only the few selectors that explicitly use font-family: var(--font-sans) change, so the saved UI font setting is visibly ignored across large parts of the app; route the base font utility/body rule through the runtime variable as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Body and html are not pinned to Inter. The @theme inline block maps --font-sans to the runtime :root variable, and the applier sets --font-sans inline on , which outranks the :root/.dark declarations, so the custom UI font already reaches inherited body/html text.
| const nextCustomization = sanitizeCustomization( | ||
| remote.appearance.customization, | ||
| ); |
There was a problem hiding this comment.
For accounts that already have a saved personalization record from before this field existed, the GET response is still saved but appearance.customization is filled with defaults by the response model. This branch then treats that default as authoritative and calls replaceAll, so enabling sync or reloading after local font/color changes silently resets them and then marks that default as last saved; detect missing/older fields or merge with local state before overwriting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good point. A record saved before the customization field existed came back with it server-defaulted, and the client treated that as authoritative and wiped local overrides. The GET now returns customizationSaved, and the client keeps and re-pushes local customization for legacy records while an explicit reset (customizationSaved true) still wins. Fixed in bd1b9c7.
- Guard the theme and palette storage reads separately so a blocked localStorage (private browsing) still resolves a mode from the OS preference instead of skipping the boot entirely - Firefox makes scrollable containers keyboard focusable and drew its 3px UA outline on them; swap it for the app's soft 1px indicator
The theme block declared the sans and mono stacks as literals, so Tailwind inlined them into every font-sans and font-mono utility at build time and the runtime overrides from Settings > Appearance never applied. Reference the :root tokens instead, matching how the color tokens already work.
…eanup - Move font importing into each font dropdown: Upload and Select folder sit side by side under the list, imported fonts get an inline remove, and the standalone Import font row is gone - Uploads reuse fonts the user already has (bundled, imported, or installed, matched by file name with style suffixes stripped) instead of embedding a duplicate copy; only new fonts are embedded - Folder scan lists font files from a picked folder in every dropdown for the session; picking one imports it through the same path - Fallback avatar uses the control accent with a readable foreground instead of the neutral primary that rendered black outside standard - Monitor bars, progress defaults, sliders, and usage meters use the control accent; warning and danger tiers stay amber and red - User facing strings that called the app just Studio now say Unsloth in all four locales, keeping Unsloth Studio and LM Studio intact
Upload and Select folder now read from the left like the list items, with a short vertical rule between the two.
- Sliders are controls, not meters, so their fill goes back to the neutral primary instead of the palette accent - The base h1 rule reads --font-heading with !important and the chat thread root resets that variable to the sans stack, which pulled the greeting off Hellix; restore the stack on the greeting element
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cdc9b2340
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return ( | ||
| <PopoverPrimitive.Portal> | ||
| <PopoverPrimitive.Portal | ||
| container={container ?? dialogContainer ?? undefined} |
There was a problem hiding this comment.
When these popovers are opened from the Settings dialog, dialogContainer is still null: DialogContent only provides the optional container prop, and SettingsDialog does not pass one. In that scenario the new Appearance popovers such as the font selector/color picker remain portaled to <body>, even though the comment notes that modal dialog body scroll lock swallows wheel events on body-portaled content; long lists like device fonts therefore cannot scroll reliably. Please provide the actual dialog content element here or pass a concrete container from the dialog.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Both Appearance popovers (font selector and color picker) are modal={true}, so Radix wraps each in its own RemoveScroll. react-remove-scroll only lets the topmost (last-mounted) lock act, so while the popover is open its lock is active and the dialog's is silenced, which lets the popover scroll its own font list even though it is body-portaled; I confirmed this against the installed react-remove-scroll lockStack behaviour. Providing the DialogContent element as the container would actually regress this: DialogContent is positioned with translate(-50%, -50%), and a transform establishes the containing block for the fixed-positioned Radix popper, so portaling the popover inside it would misplace it. That is exactly why these popovers are body-portaled, and the useDialogPortalContainer fallback stays as an inert safety net for any future non-modal settings popover.
| palette: Literal["standard", "classic", "minimal"] = "standard" | ||
| language: Optional[str] = Field(None, max_length = 20) | ||
| customization: PersonalizationCustomization = Field( | ||
| default_factory = PersonalizationCustomization | ||
| ) |
There was a problem hiding this comment.
When a stale/older frontend tab sends the previous personalization shape without palette or customization, these defaulted fields are materialized during validation and update_personalization_settings persists them via payload.model_dump(). The next GET then sees the keys and reports paletteSaved/customizationSaved as true, so the new client treats the defaults as explicit remote choices and can overwrite local palette/custom controls instead of preserving legacy-missing fields; merge with the existing record or persist only fields present in the request.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, this defeats the *Saved detection I added: a stale tab omitting palette/customization had those defaults materialized by model_dump() and persisted, flipping the flags to true. The PUT now dumps only the request's set fields (exclude_unset) and merges them onto the stored record, so omitted fields keep whatever was already stored and a full save still writes everything. Fixed in b9f9703.
A stale client that omits palette or customization previously had those defaults materialized by model_dump() and persisted, which flipped paletteSaved/customizationSaved to true and defeated the legacy detection. The PUT now dumps only the request's set fields and merges them onto the stored record, so omitted fields keep whatever was already stored.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The theme/palette values reach setTheme/setPalette from the authenticated personalization sync, which made the CodeQL clear-text-storage query treat writing them to localStorage as storing sensitive data. Store a re-derived literal from a constant map instead, so a plain UI preference is not tracked as sensitive; behavior is unchanged.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
- syncImportedFonts: a rejected FontFace.load() only clears the registry entry if it still points at that face, so a same-name re-import while the old load was pending is no longer untracked/leaked. - Cap imported-font names to the backend length (100) so an over-long name can no longer pass the client but fail the personalization PUT and stall sync. - Add a backend test that a stale PUT preserves an existing stored palette and customization (not just that absent fields stay absent).
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a766570c2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| animation-duration: 0.01ms !important; | ||
| animation-iteration-count: 1 !important; |
There was a problem hiding this comment.
When the in-app Reduce motion setting is On on a system whose OS preference is not reduced, this blanket rule also matches .hub-loading-bar[data-active]::after and forces its sweep animation to a single 0.01ms iteration. Fresh evidence beyond the prior loader discussion is that the Hub model catalog's loading indicator uses hub-loading-bar (studio/frontend/src/features/hub/catalog/models-catalog.tsx), while the force-reduced-motion exceptions below only cover .animate-spin, .generated-image-loading-dot, and .loading-bar-slide; the Hub exception lives only inside an OS prefers-reduced-motion media block, so this path freezes the visual loading feedback while models are being fetched.
Useful? React with 👍 / 👎.
The PUT /personalization handler returned the request payload, which Pydantic had already filled with defaults for any field the client omitted. A partial or stale write (for example a client sending only theme) therefore got back a response that contradicted both storage and the next GET: preserved fields like palette and the custom font showed their defaults instead of the stored values. Return model_validate(merged) so the response mirrors what was stored. The stored record is still the full merged dict, so legacy fields the model does not know about are preserved as before.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 710d1bb6df
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| el.classList.toggle("force-reduced-motion", c.reduceMotion === "on"); | ||
| // "off" opts out of the OS reduced-motion preference for CSS animations; | ||
| // the media rules in index.css skip html.force-motion. | ||
| el.classList.toggle("force-motion", c.reduceMotion === "off"); |
There was a problem hiding this comment.
When the OS preference is reduced and the user explicitly sets Reduce motion to Off, this only adds html.force-motion for the hand-written media rules. Fresh evidence beyond the prior discussion is that the repo still uses Tailwind motion-reduce:* utilities, e.g. motion-reduce:hidden in studio/frontend/src/features/chat/artifacts/artifact-card.tsx/artifact-surface.tsx and motion-reduce:animate-none in tool shimmer rows; those compile to plain @media (prefers-reduced-motion: reduce) selectors that do not check html.force-motion. In that environment the Off setting still hides/disables those animations, so the override is incomplete unless those utilities are replaced or redefined with the same force-motion gate.
Useful? React with 👍 / 👎.
- Settings search now scrolls to the result within its destination tab instead of a same-named row in the previously rendered deferred tab (for example "Storage" and "Models folder" appear in both General and Resources). - The reduce-motion segmented control honors its own Off/On/System choice by reading useReducedMotionConfig instead of the OS-only useReducedMotion. - The color picker saturation/value area is operable by keyboard, so the role="slider" surface responds to the arrow keys it advertises. - Profile avatars and palette cards show a visible keyboard focus ring again. - Guard the persisted appearance-customization write so a blocked or full localStorage does not throw out of a store action, matching the theme store. - Import the appearance store symbols from the settings feature barrel.
|
@codex review |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0e875412e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return ( | ||
| <PopoverPrimitive.Portal> | ||
| <PopoverPrimitive.Portal | ||
| container={container ?? dialogContainer ?? undefined} |
There was a problem hiding this comment.
In a normal SettingsDialog no container prop is passed to DialogContent, so useDialogPortalContainer() is still null here and the new Appearance popovers fall back to document.body. When the settings dialog is open, Radix's dialog scroll lock only shards the dialog content; the body-portaled font picker list is therefore treated as outside the dialog and wheel/trackpad scrolling is swallowed, so users with more fonts than fit in the max-h-64 list cannot scroll it. Please provide the dialog content element (or pass a real container) instead of falling back to body for popovers rendered inside modal dialogs.
Useful? React with 👍 / 👎.
| onPointerMove={(e) => { | ||
| if (e.buttons === 1) moveFromPointer(e); | ||
| }} |
There was a problem hiding this comment.
When the user has imported fonts, dragging this picker calls onChange on every pointer event; the parent writes through setColor, and the persisted appearance store serializes the entire customization object including importedFonts data URLs (allowed up to 4.4 MB) into localStorage for each move. In that scenario a simple color drag can synchronously rewrite multi-MB storage dozens of times per second and freeze the Appearance dialog; keep the live preview local while dragging and commit/debounce the store update instead.
Useful? React with 👍 / 👎.
Resolve the use-personalization-sync.ts conflict from main's locale preference refactor (#7076): keep the PR's palette and customization sync while adopting main's LocalePreference API (useLocalePreference, getLocalePreference, isLocalePreference, DEFAULT_LOCALE_PREFERENCE, and remoteLanguagePreference). The payload and hasLocalSettings signatures keep their palette and customization parameters, and the language value now flows through remoteLanguagePreference and isLocalePreference.
|
@codex review |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Unsloth Studio appearance walkthroughRan this branch in a live Studio build and captured the new appearance and control-restyle surface. Short walkthrough of palette switching, dark mode, the color picker and settings search:
Everything in a single view:
Confirmed on the running build:
Individual screenshotsAppearance tab (palettes plus customization)
Classic palette applied
Minimal palette applied
Dark mode with the Dark theme section
Classic palette in dark mode
In app color picker
Settings search for "font"
Restyled chat surface
|
3 participants
What this adds
Appearance settings in Unsloth previously only offered light/dark. This PR adds color palettes, a full set of appearance customization options, a restyle of the core controls (inputs, dropdowns, buttons), settings search, and a small settings reorganization.
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, and the sidebar stays the standard white in light mode. Focus and selection rings are neutral (foreground toned) in every palette, so highlighted elements, including typing boxes and the selected palette card, never take the accent color; the selected palette card is marked by its ring alone. Decorative brand greens (section cards, the connections list, the guided tour, onboarding summary tiles, selected quant chips) follow the palette accent, while meaning-carrying marks such as the hub verified badge and the HF token indicator keep the brand green in every palette through a dedicated token. 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. The theme bootstrap that used to be an inline script inindex.htmlis now an external/theme-boot.jsso it runs under the backend'sscript-src 'self'CSP.Focus and selection indicators
One indicator system across the app, derived from the border color so it adapts to every palette and mode:
Sidebar user menu customization
A new section in Settings > Appearance to customize the sidebar profile menu:
Profile picture
Accent colored meters
Monitor bars and progress fills follow the palette accent instead of the neutral primary, which rendered black in classic and minimal: the Live monitor cards and GPU device bars, the floating resource monitor, the shared Progress default, the training start overlay (its gradient is now built from the accent), the API monitor console, the chat context usage bar, the training GPU tiles, and slider fills. Warning and danger tiers stay amber and red.
Naming
User facing strings that referred to the app as just "Studio" now say "Unsloth" in all four locales (account password description, color settings description, UI font size description, System server description, and three download manager errors). The full "Unsloth Studio" product name, the "Fine-tuning Studio" feature title, and "LM Studio" are unchanged.
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)rounded-lgcorners, tight padding, accent hover rows, and a bordered search fieldSettings search
A search field sits at the top of the settings sidebar, with the tab list under a small Settings label. It filters setting names across every tab in the current language. Results are grouped by tab with the tab icon and name as the header, and clicking a result jumps to that tab and scrolls to the matching row with a brief highlight. Escape or the clear button resets the search.
Settings reorganization
Backend
PersonalizationAppearancegainspaletteandcustomizationwith strict validation (hex color patterns, font name length, size and contrast ranges, imported font data URL checks, sidebar menu item ids with dedupe and defaults,edgeFades).PersonalizationProfilegainsshowGreetingSloth.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, sidebar menu normalization, edge fades, the greeting sloth flag, and full round-trips