Skip to content

Commit

Permalink
Apply MVVM for profiles in SUI (#11877)
Browse files Browse the repository at this point in the history
Cleans up `ProfileViewModel`, `Profiles`, and `ProfilePageNavigationState` to move all of the view model responsibilities over to `ProfileViewModel`. We don't actually store the `ProfilePageNavigationState` anymore. We only use it as a way to transfer information to the new page.

- I pulled out `ProfileViewModel` into its own file to keep things cleaner. It was getting pretty big.
- The font lists are now stored in a static location in `ProfileViewModel`, which means that we can reuse the same list between pages.
- the profile pivot was also moved to the `ProfileViewModel` and stored as a static value.

✅ pivot behavior is the same
✅ font list is still populated
  • Loading branch information
carlos-zamora authored and zadjii-msft committed Mar 3, 2022
1 parent 4c5bd69 commit fe46f70
Show file tree
Hide file tree
Showing 17 changed files with 832 additions and 798 deletions.
4 changes: 2 additions & 2 deletions src/cascadia/TerminalSettingsEditor/Appearances.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
bool result{ false };
const auto currentFont{ Appearance().FontFace() };
for (const auto& font : SourceProfile().MonospaceFontList())
for (const auto& font : ProfileViewModel::MonospaceFontList())
{
if (font.LocalizedName() == currentFont)
{
Expand Down Expand Up @@ -175,7 +175,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// look for the current font in our shown list of fonts
const auto& appearanceVM{ Appearance() };
const auto appearanceFontFace{ appearanceVM.FontFace() };
const auto& currentFontList{ ShowAllFonts() ? SourceProfile().CompleteFontList() : SourceProfile().MonospaceFontList() };
const auto& currentFontList{ ShowAllFonts() ? ProfileViewModel::CompleteFontList() : ProfileViewModel::MonospaceFontList() };
IInspectable fallbackFont;
for (const auto& font : currentFontList)
{
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalSettingsEditor/Appearances.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,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;
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalSettingsEditor/GlobalAppearance.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
bool FeatureNotificationIconEnabled() const noexcept;

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.
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalSettingsEditor/Interaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
}

Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalSettingsEditor/Launch.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
}

Expand Down
20 changes: 9 additions & 11 deletions src/cascadia/TerminalSettingsEditor/MainPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
auto profileVM{ _viewModelForProfile(_settingsClone.ProfileDefaults(), _settingsClone) };
profileVM.IsBaseLayer(true);
_lastProfilesNavState = winrt::make<ProfilePageNavigationState>(profileVM,
_settingsClone.GlobalSettings().ColorSchemes(),
_lastProfilesNavState,
*this);
auto state{ winrt::make<ProfilePageNavigationState>(profileVM,
_settingsClone.GlobalSettings().ColorSchemes(),
*this) };

contentFrame().Navigate(xaml_typename<Editor::Profiles>(), _lastProfilesNavState);
contentFrame().Navigate(xaml_typename<Editor::Profiles>(), state);
}
else if (clickedItemTag == colorSchemesTag)
{
Expand All @@ -343,15 +342,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// - profile - the profile object we are getting a view of
void MainPage::_Navigate(const Editor::ProfileViewModel& profile)
{
_lastProfilesNavState = winrt::make<ProfilePageNavigationState>(profile,
_settingsClone.GlobalSettings().ColorSchemes(),
_lastProfilesNavState,
*this);
auto state{ winrt::make<ProfilePageNavigationState>(profile,
_settingsClone.GlobalSettings().ColorSchemes(),
*this) };

// Add an event handler for when the user wants to delete a profile.
_lastProfilesNavState.DeleteProfile({ this, &MainPage::_DeleteProfile });
profile.DeleteProfile({ this, &MainPage::_DeleteProfile });

contentFrame().Navigate(xaml_typename<Editor::Profiles>(), _lastProfilesNavState);
contentFrame().Navigate(xaml_typename<Editor::Profiles>(), state);
}

void MainPage::OpenJsonTapped(IInspectable const& /*sender*/, Windows::UI::Xaml::Input::TappedRoutedEventArgs const& /*args*/)
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalSettingsEditor/MainPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void _Navigate(const Editor::ProfileViewModel& profile);

winrt::Microsoft::Terminal::Settings::Editor::ColorSchemesPageNavigationState _colorSchemesNavState{ nullptr };
winrt::Microsoft::Terminal::Settings::Editor::ProfilePageNavigationState _lastProfilesNavState{ nullptr };
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@
<DependentUpon>Profiles.xaml</DependentUpon>
<SubType>Code</SubType>
</ClInclude>
<ClInclude Include="ProfileViewModel.h">
<DependentUpon>ProfileViewModel.idl</DependentUpon>
<SubType>Code</SubType>
</ClInclude>
<ClInclude Include="Appearances.h">
<DependentUpon>Appearances.xaml</DependentUpon>
<SubType>Code</SubType>
Expand Down Expand Up @@ -173,6 +177,10 @@
<DependentUpon>Profiles.xaml</DependentUpon>
<SubType>Code</SubType>
</ClCompile>
<ClCompile Include="ProfileViewModel.cpp">
<DependentUpon>ProfileViewModel.idl</DependentUpon>
<SubType>Code</SubType>
</ClCompile>
<ClCompile Include="Appearances.cpp">
<DependentUpon>Appearances.xaml</DependentUpon>
<SubType>Code</SubType>
Expand Down Expand Up @@ -238,6 +246,7 @@
<DependentUpon>Profiles.xaml</DependentUpon>
<SubType>Code</SubType>
</Midl>
<Midl Include="ProfileViewModel.idl" />
<Midl Include="Appearances.idl">
<DependentUpon>Appearances.xaml</DependentUpon>
<SubType>Code</SubType>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<ClInclude Include="PreviewConnection.h" />
</ItemGroup>
<ItemGroup>
<Midl Include="ProfileViewModel.idl" />
<Midl Include="EnumEntry.idl" />
<Midl Include="Converters.idl">
<Filter>Converters</Filter>
Expand Down
Loading

0 comments on commit fe46f70

Please sign in to comment.