Skip to content

feat(settings): preview terminal font options#2186

Merged
arnestrickmann merged 2 commits into
mainfrom
jan/eng-1316-render-each-font-option-in-its-own-font
May 24, 2026
Merged

feat(settings): preview terminal font options#2186
arnestrickmann merged 2 commits into
mainfrom
jan/eng-1316-render-each-font-option-in-its-own-font

Conversation

@janburzinski
Copy link
Copy Markdown
Collaborator

image

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 22, 2026

Greptile Summary

This PR adds inline font previews to the terminal font picker dropdown by wrapping each option label in a <span> styled with the corresponding fontFamily. The default option falls back to 'Menlo' since its FontOption.value is an empty string.

  • The font name 'Menlo' is now written in two places: embedded in DEFAULT_OPTION.label and as the fallback in the new inline style — introducing a subtle maintenance coupling where changing the default font name requires updating both independently.
  • The change is narrowly scoped to the dropdown item renderer and has no effect on how fonts are saved or applied to the terminal.

Confidence Score: 4/5

Safe to merge; the change is a one-line UI addition with no impact on how fonts are persisted or applied.

The only concern is the duplicated 'Menlo' literal — it appears in both DEFAULT_OPTION.label and the new inline style fallback, so a future rename would need two edits to stay consistent. Everything else about the change is straightforward.

src/renderer/features/settings/components/TerminalSettingsCard.tsx — the duplicated default font name literal.

Important Files Changed

Filename Overview
src/renderer/features/settings/components/TerminalSettingsCard.tsx Wraps each font dropdown label in a that sets its own fontFamily to preview the font; minor duplication of the hardcoded 'Menlo' default name.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ComboboxCollection renders FontOption] --> B{item.value truthy?}
    B -->|yes| C[fontFamily set to quoted item.value]
    B -->|no - DEFAULT_OPTION| D[fontFamily set to hardcoded Menlo]
    C --> E[span renders label in selected font]
    D --> E
Loading

Comments Outside Diff (1)

  1. src/renderer/features/settings/components/TerminalSettingsCard.tsx, line 49-52 (link)

    P2 The default font name 'Menlo' is hardcoded here but is already embedded in DEFAULT_OPTION.label ('Default (Menlo)'). If the default font changes, this fallback preview will silently show the wrong font. Extracting it to a shared constant keeps both in sync.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/renderer/features/settings/components/TerminalSettingsCard.tsx
    Line: 49-52
    
    Comment:
    The default font name `'Menlo'` is hardcoded here but is already embedded in `DEFAULT_OPTION.label` (`'Default (Menlo)'`). If the default font changes, this fallback preview will silently show the wrong font. Extracting it to a shared constant keeps both in sync.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/renderer/features/settings/components/TerminalSettingsCard.tsx:49-52
The default font name `'Menlo'` is hardcoded here but is already embedded in `DEFAULT_OPTION.label` (`'Default (Menlo)'`). If the default font changes, this fallback preview will silently show the wrong font. Extracting it to a shared constant keeps both in sync.

```suggestion
const DEFAULT_FONT_FAMILY = 'Menlo';

const DEFAULT_OPTION: FontOption = {
  value: '',
  label: `Default (${DEFAULT_FONT_FAMILY})`,
};
```

### Issue 2 of 2
src/renderer/features/settings/components/TerminalSettingsCard.tsx:206
Once the shared constant exists, the fallback in the inline style should reference it instead of repeating the literal `'Menlo'`.

```suggestion
                            <span style={{ fontFamily: item.value ? `"${item.value}"` : DEFAULT_FONT_FAMILY }}>
```

Reviews (1): Last reviewed commit: "feat(settings): preview terminal font op..." | Re-trigger Greptile

Comment thread src/renderer/features/settings/components/TerminalSettingsCard.tsx Outdated
Copy link
Copy Markdown
Contributor

@arnestrickmann arnestrickmann left a comment

Choose a reason for hiding this comment

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

nice!!

@arnestrickmann
Copy link
Copy Markdown
Contributor

Nice!

CleanShot 2026-05-23 at 18 48 03@2x

@arnestrickmann arnestrickmann merged commit 5108d0f into main May 24, 2026
1 check passed
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