Skip to content

Commit

Permalink
Add a warning when a proportional font is selected (#15195)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Add an infobar warning when a non-monospaced font is selected.
## References and Relevant Issues
#13389 
## Detailed Description of the Pull Request / Additional comments
I initially had the `IsOpen` property of the infobar bound to the
`ShowAllFonts` checkbox property. However, I felt we could do better by
adding a property for it since there was already a method defined to
inspect whether the selected font was in the `MonoSpaceFontList`.
## Validation Steps Performed
Warning shows up when a non-monospaced font is selected either globally
or on individual profiles. All existing tests continue to pass.
<img width="868" alt="image"
src="https://user-images.githubusercontent.com/2086722/232594214-cd42397b-ce9d-499c-aa73-3feaa45e850e.png">

## PR Checklist
- [x] Closes #13389 
- [x] Tests added/passed
- [ ] Documentation updated
- If checked, please file a pull request on [our docs
repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
- [ ] Schema updated (if necessary)
  • Loading branch information
jamespack authored Apr 20, 2023
1 parent 0e86ce5 commit 2c16543
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 2 deletions.
17 changes: 16 additions & 1 deletion src/cascadia/TerminalSettingsEditor/Appearances.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
DependencyProperty Appearances::_AppearanceProperty{ nullptr };

Appearances::Appearances() :
_ShowAllFonts{ false }
_ShowAllFonts{ false },
_ShowProportionalFontWarning{ false }
{
InitializeComponent();

Expand Down Expand Up @@ -315,6 +316,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
const auto selectedItem{ e.AddedItems().GetAt(0) };
const auto newFontFace{ unbox_value<Editor::Font>(selectedItem) };
Appearance().FontFace(newFontFace.LocalizedName());
if (!UsingMonospaceFont())
{
ShowProportionalFontWarning(true);
}
else
{
ShowProportionalFontWarning(false);
}
}

void Appearances::_ViewModelChanged(const DependencyObject& d, const DependencyPropertyChangedEventArgs& /*args*/)
Expand Down Expand Up @@ -376,6 +385,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentAdjustIndistinguishableColors" });
}
else if (settingName == L"ShowProportionalFontWarning")
{
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"ShowProportionalFontWarning" });
}
// YOU THERE ADDING A NEW APPEARANCE SETTING
// Make sure you add a block like
//
Expand Down Expand Up @@ -407,6 +420,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"UsingMonospaceFont" });
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentIntenseTextStyle" });
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentAdjustIndistinguishableColors" });
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"ShowProportionalFontWarning" });
}
}

Expand Down Expand Up @@ -487,4 +501,5 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// whereas SelectedItem identifies which one was selected by the user.
return FontWeightComboBox().SelectedItem() == _CustomFontWeight;
}

}
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Appearances.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
DEPENDENCY_PROPERTY(Editor::AppearanceViewModel, Appearance);
WINRT_PROPERTY(Editor::ProfileViewModel, SourceProfile, nullptr);
WINRT_PROPERTY(IHostedInWindow, WindowRoot, nullptr);

GETSET_BINDABLE_ENUM_SETTING(BackgroundImageStretchMode, Windows::UI::Xaml::Media::Stretch, Appearance().BackgroundImageStretchMode);

GETSET_BINDABLE_ENUM_SETTING(IntenseTextStyle, Microsoft::Terminal::Settings::Model::IntenseStyle, Appearance().IntenseTextStyle);
WINRT_OBSERVABLE_PROPERTY(bool, ShowProportionalFontWarning, _PropertyChangedHandlers, nullptr);

private:
bool _ShowAllFonts;
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsEditor/Appearances.idl
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ namespace Microsoft.Terminal.Settings.Editor

Boolean UsingMonospaceFont { get; };
Boolean ShowAllFonts;
Boolean ShowProportionalFontWarning;

IInspectable CurrentCursorShape;
Boolean IsVintageCursor { get; };
Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/TerminalSettingsEditor/Appearances.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,13 @@
</local:SettingContainer>

<!-- Font Face -->

<local:SettingContainer x:Uid="Profile_FontFace"
ClearSettingValue="{x:Bind Appearance.ClearFontFace}"
HasSettingValue="{x:Bind Appearance.HasFontFace, Mode=OneWay}"
SettingOverrideSource="{x:Bind Appearance.FontFaceOverrideSource, Mode=OneWay}"
Visibility="{x:Bind Appearance.IsDefault, Mode=OneWay}">

<StackPanel Margin="0,8,0,0">
<!--
Binding the ItemsSource to a separate variable that switches between the
Expand All @@ -210,6 +212,10 @@
IsEnabled="{x:Bind UsingMonospaceFont, Mode=OneWay}" />
</StackPanel>
</local:SettingContainer>
<muxc:InfoBar x:Uid="Profile_FontFace_ProportionalFontWarning"
IsOpen="{x:Bind ShowProportionalFontWarning, Mode=OneWay}"
Severity="Warning" />


<!-- Font Size -->
<local:SettingContainer x:Uid="Profile_FontSize"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1608,5 +1608,13 @@
<data name="Settings_PortableModeInfoLinkTextRun.Text" xml:space="preserve">
<value>Learn more.</value>
<comment>A hyperlink displayed near Settings_PortableModeNote.Text that the user can follow for more information.</comment>
</data>
<data name="Profile_FontFace_ProportionalFontWarning.Title" xml:space="preserve">
<value>Warning:</value>
<comment>Title for the warning info bar used when a non monospace font face is chosen to indicate that there may be visual artifacts</comment>
</data>
<data name="Profile_FontFace_ProportionalFontWarning.Message" xml:space="preserve">
<value>Choosing a non-monospaced font will likely result in visual artifacts. Use at your own discretion.</value>
<comment>Warning info bar used when a non monospace font face is chosen to indicate that there may be visual artifacts</comment>
</data>
</root>

0 comments on commit 2c16543

Please sign in to comment.