starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Fix broken reset button on some profile settings (#12275)

Browse Source 
## Summary of the Pull Request
This fixes a bug where several settings would not show the reset button. The root cause of this issue is two fold:
1. Hooking up `CurrentXXX`
   - `GETSET_BINDABLE_ENUM_SETTING` was hooked up to the **settings** model profile object instead of the **view** model profile object. Since the settings model has no `PropertyChanged` system, any changes were directly being applied to the setting, but not notifying the view model (and thus, the view, by extension) to update themselves.
   - This fix required me to slightly modify the macro. Rather than using two parameters (object and function name), I used one parameter (path to getter/setter). 
2. Responding to the `PropertyChanged` notifications
   - Now that we're actually dispatching the `PropertyChanged` notifications, we need to actually respond to them. This behavior was defined in `Profiles::OnNavigatedTo()` in the `PropertyChanged()` handler. Funny enough, that code was still there, it just didn't do anything because it was trying to notify that `Profiles::CurrentXXX` changed. This is invalid because `CurrentXXX` got moved to `ProfileViewModel`.
   - The fix here was pretty easy. Just move the property changed handler to `ProfileViewModel`'s `PropertyChanged` handler that is defined in the ctor.

## References
Bug introduced in #11877

## Validation Steps Performed
✅ Profile termination behavior
✅ Bell notification style
✅ Text antialiasing
✅ Scrollbar visibility
This commit is contained in:
Carlos Zamora
2022-01-27 16:13:17 -08:00
committed by GitHub
parent 95770ed9b2
commit 2db4cbaf51
8 changed files with 56 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/cascadia/TerminalSettingsEditor/Appearances.h
View File

@@ -131,16 +131,16 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
bool IsCustomFontWeight();
WINRT_PROPERTY(Windows::Foundation::Collections::IObservableVector<Microsoft::Terminal::Settings::Editor::EnumEntry>, FontWeightList);
GETSET_BINDABLE_ENUM_SETTING(CursorShape, Microsoft::Terminal::Core::CursorStyle, Appearance(), CursorShape);
GETSET_BINDABLE_ENUM_SETTING(CursorShape, Microsoft::Terminal::Core::CursorStyle, Appearance().CursorShape);
WINRT_PROPERTY(Windows::Foundation::Collections::IObservableVector<Model::ColorScheme>, ColorSchemeList, nullptr);
WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
DEPENDENCY_PROPERTY(Editor::AppearanceViewModel, Appearance);
WINRT_PROPERTY(Editor::ProfileViewModel, SourceProfile, nullptr);
GETSET_BINDABLE_ENUM_SETTING(BackgroundImageStretchMode, Windows::UI::Xaml::Media::Stretch, Appearance(), BackgroundImageStretchMode);
GETSET_BINDABLE_ENUM_SETTING(BackgroundImageStretchMode, Windows::UI::Xaml::Media::Stretch, Appearance().BackgroundImageStretchMode);
GETSET_BINDABLE_ENUM_SETTING(IntenseTextStyle, Microsoft::Terminal::Settings::Model::IntenseStyle, Appearance(), IntenseTextStyle);
GETSET_BINDABLE_ENUM_SETTING(IntenseTextStyle, Microsoft::Terminal::Settings::Model::IntenseStyle, Appearance().IntenseTextStyle);
private:
bool _ShowAllFonts;

4
src/cascadia/TerminalSettingsEditor/GlobalAppearance.h
View File

@@ -26,8 +26,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void OnNavigatedTo(const winrt::Windows::UI::Xaml::Navigation::NavigationEventArgs& e);
WINRT_PROPERTY(Editor::GlobalAppearancePageNavigationState, State, nullptr);
GETSET_BINDABLE_ENUM_SETTING(Theme, winrt::Windows::UI::Xaml::ElementTheme, State().Globals(), Theme);
GETSET_BINDABLE_ENUM_SETTING(TabWidthMode, winrt::Microsoft::UI::Xaml::Controls::TabViewWidthMode, State().Globals(), TabWidthMode);
GETSET_BINDABLE_ENUM_SETTING(Theme, winrt::Windows::UI::Xaml::ElementTheme, State().Globals().Theme);
GETSET_BINDABLE_ENUM_SETTING(TabWidthMode, winrt::Microsoft::UI::Xaml::Controls::TabViewWidthMode, State().Globals().TabWidthMode);
public:
// LanguageDisplayConverter maps the given BCP 47 tag to a localized string.

4
src/cascadia/TerminalSettingsEditor/Interaction.h
View File

@@ -26,8 +26,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_PROPERTY(Editor::InteractionPageNavigationState, State, nullptr);
GETSET_BINDABLE_ENUM_SETTING(TabSwitcherMode, Model::TabSwitcherMode, State().Globals(), TabSwitcherMode);
GETSET_BINDABLE_ENUM_SETTING(CopyFormat, winrt::Microsoft::Terminal::Control::CopyFormat, State().Globals(), CopyFormatting);
GETSET_BINDABLE_ENUM_SETTING(TabSwitcherMode, Model::TabSwitcherMode, State().Globals().TabSwitcherMode);
GETSET_BINDABLE_ENUM_SETTING(CopyFormat, winrt::Microsoft::Terminal::Control::CopyFormat, State().Globals().CopyFormatting);
};
}

6
src/cascadia/TerminalSettingsEditor/Launch.h
View File

@@ -33,9 +33,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_PROPERTY(Editor::LaunchPageNavigationState, State, nullptr);
GETSET_BINDABLE_ENUM_SETTING(FirstWindowPreference, Model::FirstWindowPreference, State().Settings().GlobalSettings(), FirstWindowPreference);
GETSET_BINDABLE_ENUM_SETTING(LaunchMode, Model::LaunchMode, State().Settings().GlobalSettings(), LaunchMode);
GETSET_BINDABLE_ENUM_SETTING(WindowingBehavior, Model::WindowingMode, State().Settings().GlobalSettings(), WindowingBehavior);
GETSET_BINDABLE_ENUM_SETTING(FirstWindowPreference, Model::FirstWindowPreference, State().Settings().GlobalSettings().FirstWindowPreference);
GETSET_BINDABLE_ENUM_SETTING(LaunchMode, Model::LaunchMode, State().Settings().GlobalSettings().LaunchMode);
GETSET_BINDABLE_ENUM_SETTING(WindowingBehavior, Model::WindowingMode, State().Settings().GlobalSettings().WindowingBehavior);
};
}

16
src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp
View File

@@ -115,6 +115,22 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
Opacity(1.0);
}
}
else if (viewModelProperty == L"AntialiasingMode")
{
_NotifyChanges(L"CurrentAntiAliasingMode");
}
else if (viewModelProperty == L"CloseOnExit")
{
_NotifyChanges(L"CurrentCloseOnExitMode");
}
else if (viewModelProperty == L"BellStyle")
{
_NotifyChanges(L"IsBellStyleFlagSet");
}
else if (viewModelProperty == L"ScrollState")
{
_NotifyChanges(L"CurrentScrollState");
}
});
// Do the same for the starting directory

12
src/cascadia/TerminalSettingsEditor/ProfileViewModel.h
View File

@@ -77,12 +77,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void DeleteUnfocusedAppearance();
bool AtlasEngineAvailable() const noexcept;
WINRT_PROPERTY(bool, IsBaseLayer, false);
WINRT_PROPERTY(IHostedInWindow, WindowRoot, nullptr);
GETSET_BINDABLE_ENUM_SETTING(AntiAliasingMode, Microsoft::Terminal::Control::TextAntialiasingMode, _profile, AntialiasingMode);
GETSET_BINDABLE_ENUM_SETTING(CloseOnExitMode, Microsoft::Terminal::Settings::Model::CloseOnExitMode, _profile, CloseOnExit);
GETSET_BINDABLE_ENUM_SETTING(ScrollState, Microsoft::Terminal::Control::ScrollbarState, _profile, ScrollState);
PERMANENT_OBSERVABLE_PROJECTED_SETTING(_profile, Guid);
PERMANENT_OBSERVABLE_PROJECTED_SETTING(_profile, ConnectionType);
OBSERVABLE_PROJECTED_SETTING(_profile, Name);
@@ -111,6 +105,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
OBSERVABLE_PROJECTED_SETTING(_profile, UseAtlasEngine);
OBSERVABLE_PROJECTED_SETTING(_profile, Elevate);
WINRT_PROPERTY(bool, IsBaseLayer, false);
WINRT_PROPERTY(IHostedInWindow, WindowRoot, nullptr);
GETSET_BINDABLE_ENUM_SETTING(AntiAliasingMode, Microsoft::Terminal::Control::TextAntialiasingMode, AntialiasingMode);
GETSET_BINDABLE_ENUM_SETTING(CloseOnExitMode, Microsoft::Terminal::Settings::Model::CloseOnExitMode, CloseOnExit);
GETSET_BINDABLE_ENUM_SETTING(ScrollState, Microsoft::Terminal::Control::ScrollbarState, ScrollState);
TYPED_EVENT(DeleteProfile, Editor::ProfileViewModel, Editor::DeleteProfileEventArgs);
private:

19
src/cascadia/TerminalSettingsEditor/Profiles.cpp
View File

@@ -56,24 +56,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// Subscribe to some changes in the view model
// These changes should force us to update our own set of "Current<Setting>" members,
// and propagate those changes to the UI
_ViewModelChangedRevoker = _Profile.PropertyChanged(winrt::auto_revoke, [=](auto&&, const PropertyChangedEventArgs& args) {
const auto settingName{ args.PropertyName() };
if (settingName == L"AntialiasingMode")
{
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentAntiAliasingMode" });
}
else if (settingName == L"CloseOnExit")
{
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentCloseOnExitMode" });
}
else if (settingName == L"BellStyle")
{
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"IsBellStyleFlagSet" });
}
else if (settingName == L"ScrollState")
{
_PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentScrollState" });
}
_ViewModelChangedRevoker = _Profile.PropertyChanged(winrt::auto_revoke, [=](auto&&, auto&&) {
_previewControl.UpdateControlSettings(_Profile.TermSettings(), _Profile.TermSettings());
});

46
src/cascadia/TerminalSettingsEditor/Utils.h
View File

@@ -49,29 +49,29 @@
// of EnumEntries so that we may display all possible values of the given
// enum type and its localized names. It also provides a getter and setter
// for the setting we wish to bind to.
#define GETSET_BINDABLE_ENUM_SETTING(name, enumType, settingsModelName, settingNameInModel) \
public: \
winrt::Windows::Foundation::Collections::IObservableVector<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry> name##List() \
{ \
return _##name##List; \
} \
\
winrt::Windows::Foundation::IInspectable Current##name() \
{ \
return winrt::box_value<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry>(_##name##Map.Lookup(settingsModelName.settingNameInModel())); \
} \
\
void Current##name(const winrt::Windows::Foundation::IInspectable& enumEntry) \
{ \
if (auto ee = enumEntry.try_as<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry>()) \
{ \
auto setting = winrt::unbox_value<enumType>(ee.EnumValue()); \
settingsModelName.settingNameInModel(setting); \
} \
} \
\
private: \
winrt::Windows::Foundation::Collections::IObservableVector<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry> _##name##List; \
#define GETSET_BINDABLE_ENUM_SETTING(name, enumType, viewModelSettingGetSet) \
public: \
winrt::Windows::Foundation::Collections::IObservableVector<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry> name##List() \
{ \
return _##name##List; \
} \
\
winrt::Windows::Foundation::IInspectable Current##name() \
{ \
return winrt::box_value<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry>(_##name##Map.Lookup(viewModelSettingGetSet())); \
} \
\
void Current##name(const winrt::Windows::Foundation::IInspectable& enumEntry) \
{ \
if (auto ee = enumEntry.try_as<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry>()) \
{ \
auto setting = winrt::unbox_value<enumType>(ee.EnumValue()); \
viewModelSettingGetSet(setting); \
} \
} \
\
private: \
winrt::Windows::Foundation::Collections::IObservableVector<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry> _##name##List; \
winrt::Windows::Foundation::Collections::IMap<enumType, winrt::Microsoft::Terminal::Settings::Editor::EnumEntry> _##name##Map;
// This macro defines a dependency property for a WinRT class.