#4340

fix(ui): honor the default font in showFontPicker on the desktop

Opensabrimetewants to merge intofrom

Summary

puter.ui.showFontPicker(defaultFont) and showFontPicker({ defaultFont }) preselect the font on websites but not inside Puter apps, and the string form never resolves at all on the desktop.

Why. Three things in the desktop path:

  1. The showFontPicker IPC handler assigned window_options onto event.data.options without first normalizing a string argument, so the documented showFontPicker('Georgia') form threw on property assignment and the app never received fontPicked. The showColorPicker handler right below already normalizes strings.
  2. UIWindowFontPicker only read options.default, so defaultFont (the documented option) and defaultValue (the alias the color picker accepts) were ignored.
  3. onAppend: function (window) shadowed the global window, so window.scrollParentToChild(...) threw whenever a font actually was preselected and the Select button handlers were never attached. This was unreachable before because preselection never matched.

What. IPC normalizes a string to { defaultFont } the same way the color picker does. The picker window resolves defaultValue ?? defaultFont ?? default (same precedence as UIWindowColorPicker) and matches the name case-insensitively, as the puter.js PuterFontPicker web component already does, so both environments behave the same. The onAppend parameter is renamed so the global is reachable. The SDK standalone fallback accepts defaultValue too, the JSDoc mirrors showColorPicker, and the docs page documents the options object and the return value.

Testing

  • New Playwright spec tests/e2e/specs/showFontPicker.spec.js with fixture font-picker.html, covering the string, { defaultFont } and lowercase { defaultValue } forms in an app: the picker window opens, exactly one .font-selector-active entry is Georgia, and Select resolves { fontFamily: 'Georgia' } in the app. All 6 cases (chromium and mobile projects) fail before the fix and pass after.
  • npm run check:puterjs:types passes.
  • Verified manually on a local npm start desktop.

Activity

5
CLAassistant5d ago


All committers have signed the CLA.

udit191993d ago

Thanks for this. I looked at the same bug and found a few gaps you might want to cover here:

  1. Cancel result isn't in the types or docs. If the user closes the picker without choosing, the promise resolves false inside Puter and null on a website. I checked both against a local Puter. The JSDoc still says Promise<{ fontFamily: string }>, the Return value section doesn't mention cancelling, and the docs example (and playground/examples/ui-show-font-picker.html) reads font.fontFamily without checking, which throws on null.
  2. No @overload blocks. AGENTS.md asks for one @overload per call form on public puter.js methods, so showFontPicker(defaultFont?: string) and showFontPicker(options?: FontPickerOptions).
  3. Docs don't give the alias order. They say defaultValue and default are aliases, but in the code defaultValue wins over defaultFont. Stating the order would help.
  4. The test doesn't check scrolling. Georgia sits in the middle of the list. Preselecting a font near the end (e.g. Verdana) and asserting toBeInViewport() on .font-selector-active would cover the scrollParentToChild path the onAppend fix restores. A case that closes the window and expects false would cover point 1.
sabrimete3d ago

Thanks, I checked each point against the code and all four hold. Addressed in 31b9bb5:

  1. Cancel result. Confirmed: the desktop window resolves false on close and IPC forwards it as-is, while the web component emits null. I kept the runtime as it is (changing either value would break callers that already check for it) and made the types and docs tell the truth instead: the return type is now Promise<{ fontFamily: string } | false | null>, the Return value section states both cases, and the docs example and playground example guard the result before reading fontFamily.
  2. @overload blocks. Added one for the string form and one for the options form, following the launchApp pattern in the same file. npm run check:puterjs:types is clean.
  3. Alias order. Documented: defaultValue, then defaultFont, then default, in both the JSDoc and the docs page.
  4. Tests. Added a Verdana case asserting toBeInViewport() on the active entry, and a close-without-selecting case expecting false. I verified the scroll assertion is meaningful by removing the onAppend body locally: that test fails with viewport ratio 0 and passes with it restored. Spec is at 10/10 across both Playwright projects.