Skip to content

fix(settings): persist font size changes in Appearance theme editor#1415

Merged
datlechin merged 2 commits into
TableProApp:mainfrom
shouwang0527:zhaojy/debug-theme-font-size
May 25, 2026
Merged

fix(settings): persist font size changes in Appearance theme editor#1415
datlechin merged 2 commits into
TableProApp:mainfrom
shouwang0527:zhaojy/debug-theme-font-size

Conversation

@shouwang0527
Copy link
Copy Markdown
Contributor

Fixes #1381.

Summary

Two-file fix for the font size bug:

  1. ThemeEngine.saveUserTheme now upserts the saved theme into availableThemes synchronously before kicking off reloadAvailableThemes(). The previous code relied on the async reload completing before the SwiftUI binding setter ran, so the new (Custom) theme was missing from the list on the same tick and the picker silently reverted to the original built-in.
  2. ThemeEditorFontsSection.updateFont no longer calls engine.activateTheme(copy) directly. Activation goes through the onThemeDuplicated callback, which also writes preferredLightThemeId / preferredDarkThemeId. Without that, relaunch reconciliation reverted to the built-in theme and the font size change was lost.

Together these fix all three symptoms in the issue: change applies immediately, persists across relaunch, and no orphan (Custom) themes accumulate per adjustment.

Test plan

  • Open Settings > Appearance > Fonts, change Editor Font Size from 13 to 18, confirm SQL editor updates immediately.
  • Quit and relaunch, confirm size is still 18 and the active theme is the saved (Custom) theme.
  • Repeat the size change and confirm only one (Custom) theme exists in ~/Library/Application Support/TablePro/Themes/.

Local build skipped (no Xcode locally), relying on CI.

@github-actions
Copy link
Copy Markdown
Contributor

Thank you for your contribution! Before we can merge this PR, you need to sign our Contributor License Agreement.

To sign, please comment below with:

I have read the CLA Document and I hereby sign the CLA.


I have read the CLA Document and I hereby sign the CLA.


zhaojianyong seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
Copy link
Copy Markdown
Member

@datlechin datlechin left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@datlechin datlechin merged commit 8e64bb0 into TableProApp:main May 25, 2026
1 check failed
@shouwang0527
Copy link
Copy Markdown
Contributor Author

Thanks @datlechin for the quick review and merge!

Since this fixes a regression that makes the editor font feel too small and any adjustment in Appearance > Fonts gets silently reverted, it's a noticeable papercut for daily use. Would it be possible to include this in the next release? Happy to help verify a pre-release build if that's useful.

Thanks again for maintaining TablePro.

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.

Font size changes in Appearance settings do not persist or take effect

2 participants