-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Allow creating and editing unfocused appearances in the SUI #10317
Changes from all commits
b923b4b
85a6392
d750d56
ad35c70
5920408
7eea902
f5078b8
f6cf9dd
0800d64
81a8b8b
f179248
c8ee89f
62db2fa
5f047a1
6c7d997
3c3fede
05666f2
ef71e80
3dca0dd
e384b00
8e604ba
9ac1407
901d4bb
7e0264d
f5b5bb9
fbfdb2d
6177c26
e04d9fa
720d715
6aab43a
3e97289
8631907
0d8649c
10dda55
fd71d30
8b8cf55
9a6d580
5c4f5d6
eb9cb96
b6698e6
38f761d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | |
_profile{ profile }, | ||
_defaultAppearanceViewModel{ winrt::make<implementation::AppearanceViewModel>(profile.DefaultAppearance().try_as<AppearanceConfig>()) }, | ||
_originalProfileGuid{ profile.Guid() }, | ||
_appSettings{ appSettings } | ||
_appSettings{ appSettings }, | ||
_unfocusedAppearanceViewModel{ nullptr } | ||
{ | ||
// Add a property changed handler to our own property changed event. | ||
// This propagates changes from the settings model to anybody listening to our | ||
|
@@ -63,6 +64,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | |
{ | ||
UpdateFontList(); | ||
} | ||
|
||
if (profile.HasUnfocusedAppearance()) | ||
{ | ||
_unfocusedAppearanceViewModel = winrt::make<implementation::AppearanceViewModel>(profile.UnfocusedAppearance().try_as<AppearanceConfig>()); | ||
} | ||
|
||
_defaultAppearanceViewModel.IsDefault(true); | ||
} | ||
|
||
Model::TerminalSettings ProfileViewModel::TermSettings() const | ||
|
@@ -239,6 +247,51 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | |
return _defaultAppearanceViewModel; | ||
} | ||
|
||
bool ProfileViewModel::HasUnfocusedAppearance() | ||
{ | ||
return _profile.HasUnfocusedAppearance(); | ||
} | ||
|
||
bool ProfileViewModel::EditableUnfocusedAppearance() | ||
{ | ||
if constexpr (Feature_EditableUnfocusedAppearance::IsEnabled()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the same as "return Feature_x::IsEnabled()" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious: isn't this technically faster because of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. The compiler is smart enough to optimize...
into
on its own. |
||
{ | ||
return true; | ||
} | ||
return false; | ||
} | ||
|
||
bool ProfileViewModel::ShowUnfocusedAppearance() | ||
{ | ||
return EditableUnfocusedAppearance() && HasUnfocusedAppearance(); | ||
} | ||
|
||
void ProfileViewModel::CreateUnfocusedAppearance(const Windows::Foundation::Collections::IMapView<hstring, Model::ColorScheme>& schemes, | ||
const IHostedInWindow& windowRoot) | ||
{ | ||
_profile.CreateUnfocusedAppearance(); | ||
|
||
_unfocusedAppearanceViewModel = winrt::make<implementation::AppearanceViewModel>(_profile.UnfocusedAppearance().try_as<AppearanceConfig>()); | ||
_unfocusedAppearanceViewModel.Schemes(schemes); | ||
_unfocusedAppearanceViewModel.WindowRoot(windowRoot); | ||
|
||
_NotifyChanges(L"UnfocusedAppearance", L"HasUnfocusedAppearance"); | ||
} | ||
|
||
void ProfileViewModel::DeleteUnfocusedAppearance() | ||
{ | ||
_profile.DeleteUnfocusedAppearance(); | ||
|
||
_unfocusedAppearanceViewModel = nullptr; | ||
|
||
_NotifyChanges(L"HasUnfocusedAppearance"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you not also need to notify There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The control's visibility gets set to false from the |
||
} | ||
|
||
Editor::AppearanceViewModel ProfileViewModel::UnfocusedAppearance() | ||
{ | ||
return _unfocusedAppearanceViewModel; | ||
} | ||
|
||
bool ProfileViewModel::UseParentProcessDirectory() | ||
{ | ||
return StartingDirectory().empty(); | ||
|
@@ -291,6 +344,16 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | |
_DeleteProfileHandlers(*this, *deleteProfileArgs); | ||
} | ||
|
||
void ProfilePageNavigationState::CreateUnfocusedAppearance() | ||
{ | ||
_Profile.CreateUnfocusedAppearance(_Schemes, _WindowRoot); | ||
} | ||
|
||
void ProfilePageNavigationState::DeleteUnfocusedAppearance() | ||
{ | ||
_Profile.DeleteUnfocusedAppearance(); | ||
} | ||
|
||
PankajBhojwani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Profiles::Profiles() : | ||
_previewControl{ Control::TermControl(Model::TerminalSettings{}, make<PreviewConnection>()) } | ||
{ | ||
|
@@ -423,6 +486,16 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | |
state->DeleteProfile(); | ||
} | ||
|
||
void Profiles::CreateUnfocusedAppearance_Click(IInspectable const& /*sender*/, RoutedEventArgs const& /*e*/) | ||
{ | ||
_State.CreateUnfocusedAppearance(); | ||
} | ||
|
||
void Profiles::DeleteUnfocusedAppearance_Click(IInspectable const& /*sender*/, RoutedEventArgs const& /*e*/) | ||
{ | ||
_State.DeleteUnfocusedAppearance(); | ||
} | ||
|
||
fire_and_forget Profiles::Icon_Click(IInspectable const&, RoutedEventArgs const&) | ||
{ | ||
auto lifetime = get_strong(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,13 @@ namespace Microsoft.Terminal.Settings.Editor | |
Boolean UseCustomStartingDirectory { get; }; | ||
AppearanceViewModel DefaultAppearance { get; }; | ||
Guid OriginalProfileGuid { get; }; | ||
Boolean HasUnfocusedAppearance { get; }; | ||
Boolean EditableUnfocusedAppearance { get; }; | ||
Boolean ShowUnfocusedAppearance { get; }; | ||
AppearanceViewModel UnfocusedAppearance { get; }; | ||
|
||
void CreateUnfocusedAppearance(Windows.Foundation.Collections.IMapView<String, Microsoft.Terminal.Settings.Model.ColorScheme> Schemes, IHostedInWindow WindowRoot); | ||
void DeleteUnfocusedAppearance(); | ||
|
||
OBSERVABLE_PROJECTED_PROFILE_SETTING(String, Name); | ||
PERMANENT_OBSERVABLE_PROJECTED_SETTING(Guid, Guid); | ||
|
@@ -74,6 +81,9 @@ namespace Microsoft.Terminal.Settings.Editor | |
ProfileViewModel Profile; | ||
ProfilesPivots LastActivePivot; | ||
|
||
void CreateUnfocusedAppearance(); | ||
void DeleteUnfocusedAppearance(); | ||
|
||
Comment on lines
+84
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can probably remove these two functions and just go through the profile instead, right? These don't feel like they should be a part of the navigation state. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately we need some way to pass the list of color schemes and the window root to the profile view model for it to create an appearance view model. The PVM doesn't have the list of color schemes nor the window root, only the navigation state does. |
||
event Windows.Foundation.TypedEventHandler<ProfilePageNavigationState, DeleteProfileEventArgs> DeleteProfile; | ||
}; | ||
|
||
|
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.
Hmm... could we somehow detect something like "AppearanceChanged"?
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.
That's what's happening here already! (Note that we only get here from the
_ViewModelChanged
event)The thing is the
Appearances
object (not the view model) has some settings that xaml binds to (likeCurrentColorScheme
) that hang around even if the view model is deleted, then when a new view model is created we need to tell xaml that these settings may have changed