-
-
Notifications
You must be signed in to change notification settings - Fork 368
Import Theme Size / Select Default Font / Small Design Fixes #3458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Import Theme Size / Select Default Font / Small Design Fixes #3458
Conversation
Change showPlaceholder to true from false
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…50412-ImportThemePreset
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Adjust Colors
This comment has been minimized.
This comment has been minimized.
- Adjust themes
This comment has been minimized.
This comment has been minimized.
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
Length of output: 1065 🏁 Script executed:
Length of output: 10037 🏁 Script executed:
Length of output: 40462 @onesounds Looking at this pull request, I can see you've made several significant enhancements to improve the font handling and theme customization in Flow Launcher. The key improvements include:
These changes should significantly improve the user experience, especially addressing the pixelated text and blurry screenshots issues you mentioned in the PR description. The font selection prioritizing Noto Sans with language-specific variants is a particularly good approach for WPF compatibility. Very solid work on enhancing both the functionality and aesthetics of Flow Launcher! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 3 out of 15 changed files in this pull request and generated 2 comments.
Files not reviewed (12)
- Flow.Launcher/Languages/en.xaml: Language not supported
- Flow.Launcher/Resources/CustomControlTemplate.xaml: Language not supported
- Flow.Launcher/Resources/Dark.xaml: Language not supported
- Flow.Launcher/Resources/Light.xaml: Language not supported
- Flow.Launcher/ResultListBox.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml: Language not supported
- Flow.Launcher/SettingWindow.xaml: Language not supported
- Flow.Launcher/Themes/Base.xaml: Language not supported
- Flow.Launcher/Themes/Circle System.xaml: Language not supported
- Flow.Launcher/Themes/SlimLight.xaml: Language not supported
- Flow.Launcher/Themes/Win11Light.xaml: Language not supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the default font settings and improves theme import functionality to enhance UI clarity and ensure fonts are optimized for WPF. Key changes include:
- Replacing hard-coded default fonts with a system-derived default using language-specific mapping.
- Reducing the default QueryBoxFontSize from 20 to 16.
- Adding an "Import Theme Size" command that loads size values from the active theme's resource dictionary.
Reviewed Changes
Copilot reviewed 3 out of 15 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
Flow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.cs | Updates default font handling and introduces the Import command. |
Flow.Launcher.Infrastructure/UserSettings/Settings.cs | Adjusts default font settings and QueryBoxFontSize, and adds language-based font mapping. |
Flow.Launcher.Core/Resource/Theme.cs | Exposes the current resource dictionary and introduces a style-copy helper. |
Files not reviewed (12)
- Flow.Launcher/Languages/en.xaml: Language not supported
- Flow.Launcher/Resources/CustomControlTemplate.xaml: Language not supported
- Flow.Launcher/Resources/Dark.xaml: Language not supported
- Flow.Launcher/Resources/Light.xaml: Language not supported
- Flow.Launcher/ResultListBox.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml: Language not supported
- Flow.Launcher/SettingWindow.xaml: Language not supported
- Flow.Launcher/Themes/Base.xaml: Language not supported
- Flow.Launcher/Themes/Circle System.xaml: Language not supported
- Flow.Launcher/Themes/SlimLight.xaml: Language not supported
- Flow.Launcher/Themes/Win11Light.xaml: Language not supported
Comments suppressed due to low confidence (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.cs:26
- [nitpick] Consider marking DefaultFont as readonly if its value is not intended to be modified after initialization.
private string DefaultFont = Settings.GetSystemDefaultFont();
This comment has been minimized.
This comment has been minimized.
…ds/Flow.Launcher into 250412-ImportThemePreset
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the theme and font initialization logic to improve UI clarity and design consistency in Flow Launcher. Key changes include:
- Changing the default font source to use the OS's default and mapping language-specific fonts.
- Introducing an Import Theme Size feature that updates UI element sizes from a theme.
- Minor design fixes and adjustments to default query font size and control styling.
Reviewed Changes
Copilot reviewed 4 out of 16 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
Flow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.cs | Updates default font retrieval and query font size, and adds the Import command for theme size values. |
Flow.Launcher.Infrastructure/Win32Helper.cs | Adds language-specific Noto Sans mapping and updates GetSystemDefaultFont logic. |
Flow.Launcher.Infrastructure/UserSettings/Settings.cs | Changes default font settings and adjusts placeholder/show fields to match new design. |
Flow.Launcher.Core/Resource/Theme.cs | Alters theme resource retrieval to public, and introduces a CopyStyle method for preview style updates. |
Files not reviewed (12)
- Flow.Launcher/Languages/en.xaml: Language not supported
- Flow.Launcher/Resources/CustomControlTemplate.xaml: Language not supported
- Flow.Launcher/Resources/Dark.xaml: Language not supported
- Flow.Launcher/Resources/Light.xaml: Language not supported
- Flow.Launcher/ResultListBox.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml: Language not supported
- Flow.Launcher/SettingWindow.xaml: Language not supported
- Flow.Launcher/Themes/Base.xaml: Language not supported
- Flow.Launcher/Themes/Circle System.xaml: Language not supported
- Flow.Launcher/Themes/SlimLight.xaml: Language not supported
- Flow.Launcher/Themes/Win11Light.xaml: Language not supported
Comments suppressed due to low confidence (1)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs:310
- [nitpick] Consider explicitly initializing _hideNotifyIcon (e.g. = false) for clarity and consistency with other field declarations.
private bool _hideNotifyIcon;
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher.Infrastructure/Win32Helper.cs (1)
603-669
: Consider adding a comment explaining the font selection strategy.While the implementation is solid, adding a high-level comment at the beginning of the region would help other developers understand the rationale behind the font selection strategy, particularly why Noto Sans fonts are prioritized for different languages.
#region System Font +// This region implements a system-aware font detection strategy that prioritizes: +// 1. Language-specific Noto Sans fonts that work well with WPF's DirectWrite rendering +// 2. The system's default message font if appropriate Noto Sans isn't available +// 3. Segoe UI as a final fallback +// This approach addresses issues with pixelated text and blurry fonts reported by users. private static readonly Dictionary<string, string> _languageToNotoSans = new()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs
(6 hunks)Flow.Launcher.Infrastructure/Win32Helper.cs
(3 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.cs
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Flow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.cs
- Flow.Launcher.Infrastructure/UserSettings/Settings.cs
🔇 Additional comments (4)
Flow.Launcher.Infrastructure/Win32Helper.cs (4)
2-20
: Appropriate using statements added for new functionality.The added using statements are necessary for the new System Font functionality, including Collections.Generic for Dictionary, Linq for font queries, Markup for XmlLanguage, and the SystemFonts alias to avoid ambiguity.
605-629
: Well-structured language-to-font mapping dictionary.The dictionary comprehensively maps language codes to appropriate Noto Sans font variants for a wide range of languages, addressing the PR's goal of improving font handling across different writing systems. The static readonly approach is appropriate for this immutable mapping.
631-662
: Robust system font detection implementation with appropriate fallbacks.The implementation follows a sensible priority order:
- Try language-specific Noto Sans font if available
- Fall back to system default message font if Noto Sans isn't available
- Use "Segoe UI" as a last resort
The try-catch block ensures the method always returns a valid font name, preventing crashes in the UI.
664-667
: Concise helper method improving code readability.The
TryGetNotoFont
method encapsulates the dictionary lookup logic, making the main method cleaner and more maintainable.
What's the PR
Feature-related
Changed the default setting of the placeholder text to be enabled.
Changed the default font on first launch and reset to the OS's default font per language.
Fonts are displayed and stored based on their English names in the combo box and internally.
The previous default fonts were not suitable for WPF and caused text to appear pixelated on screen.
This is suspected to be the main reason why users are still posting blurry screenshots, even after all known issues affecting screen clarity have been fixed.
If the Noto Sans font is available, it is given priority. If not, the next default font is used. If that also fails, fall back to Segoe UI.
Added Import Theme Size feature. When the button is clicked, if the theme has specified size values, they are loaded into the settings.
Design-related
Font Issue reference
WPF uses a DirectWrite-based text rendering engine, which is different from the GDI+ rendering used in Windows Forms (System.Drawing). Because of this, font handling works differently between the two.
🔤 System.Drawing.FontFamily.GenericSansSerif vs System.Windows.Media.FontFamily
System.Drawing.FontFamily.GenericSansSerif is one of the generic font families used in GDI+, and it typically maps to fonts like Microsoft Sans Serif.
However, WPF’s System.Windows.Media.FontFamily does not support generic font families like GDI+ does. You need to specify font names directly (e.g., "Segoe UI", "Arial", etc.).
❗ Issue: Using GDI+ Fonts in WPF
Fonts like Microsoft Sans Serif are optimized for GDI+.
When used in WPF, the rendering might appear slightly blurry.
This is due to differences in subpixel rendering, DPI handling, and how text is rasterized in WPF compared to GDI+.
✅ Recommendation
Use fonts that are optimized for WPF, such as Segoe UI.
Alternatively, consider using SystemFonts.MessageFontFamily, which gives you the system’s default font for WPF.
Todo
Test Case