Skip to content

Commit

Permalink
Add functionality to open the Settings UI tab through openSettings (#…
Browse files Browse the repository at this point in the history
…7802)

This PR's goal was to add an option to the `OpenSettings` keybinding to open the Settings UI in a tab. In order to implement that, a couple of changes had to be made to `Tab`, specifically:

- Introduce a tab interface named `ITab`
- Create/Rename two new Tab classes that implement `ITab` called `SettingsTab` and `TerminalTab` 

I've also put some `TODOs` that I wanted to get some thoughts on - I'll make a follow up PR but perhaps they can be revisited when we flesh out the Settings UI more.
- Does a Settings UI tab shutdown need to do anything special for cleanup?
- Does a Settings UI tab need to have `GetActiveTitle`? Maybe depending on which page in the UI is open?
- Technically, I can't focus a `Grid` control, so I'll need to figure out what to `Focus` when the tab is selected.
- The Settings UI tab doesn't have a `TermControl`, so once focus is moved to that tab, users won't be able to `nextTab/prevTab` out of it (along with all other keybindings).

References #1564, #5915
  • Loading branch information
leonMSFT authored Oct 6, 2020
1 parent b87f6d6 commit bde5d5e
Show file tree
Hide file tree
Showing 26 changed files with 790 additions and 419 deletions.
1 change: 1 addition & 0 deletions .github/actions/spell-check/dictionary/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ IExplorer
IMap
IObject
IStorage
ITab
llabs
LCID
lround
Expand Down
6 changes: 4 additions & 2 deletions OpenConsole.sln
Original file line number Diff line number Diff line change
Expand Up @@ -2010,7 +2010,8 @@ Global
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.AuditMode|x86.Build.0 = Release|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.AuditMode|x86.Deploy.0 = Release|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Debug|Any CPU.ActiveCfg = Debug|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Debug|ARM64.ActiveCfg = Debug|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Debug|ARM64.ActiveCfg = Debug|ARM64
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Debug|ARM64.Build.0 = Debug|ARM64
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Debug|DotNet_x64Test.ActiveCfg = Debug|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Debug|DotNet_x86Test.ActiveCfg = Debug|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Debug|x64.ActiveCfg = Debug|x64
Expand All @@ -2020,7 +2021,8 @@ Global
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Debug|x86.Build.0 = Debug|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Debug|x86.Deploy.0 = Debug|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Release|Any CPU.ActiveCfg = Release|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Release|ARM64.ActiveCfg = Release|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Release|ARM64.ActiveCfg = Release|ARM64
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Release|ARM64.Build.0 = Release|ARM64
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Release|DotNet_x64Test.ActiveCfg = Release|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Release|DotNet_x86Test.ActiveCfg = Release|Win32
{CA5CAD1A-0B5E-45C3-96A8-BB496BFE4E32}.Release|x64.ActiveCfg = Release|x64
Expand Down
12 changes: 6 additions & 6 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "../TerminalApp/MinMaxCloseControl.h"
#include "../TerminalApp/TabRowControl.h"
#include "../TerminalApp/ShortcutActionDispatch.h"
#include "../TerminalApp/Tab.h"
#include "../TerminalApp/TerminalTab.h"
#include "../CppWinrtTailored.h"
#include "JsonTestClass.h"

Expand Down Expand Up @@ -246,8 +246,8 @@ namespace TerminalAppLocalTests
// In the real app, this isn't a problem, but doesn't happen
// reliably in the unit tests.
Log::Comment(L"Ensure we set the first tab as the selected one.");
auto tab{ page->_GetStrongTabImpl(0) };
page->_tabView.SelectedItem(tab->GetTabViewItem());
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
page->_tabView.SelectedItem(tab->TabViewItem());
page->_UpdatedSelectedTab(0);
});
VERIFY_SUCCEEDED(result);
Expand Down Expand Up @@ -469,7 +469,7 @@ namespace TerminalAppLocalTests

result = RunOnUIThread([&page]() {
VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
auto tab = page->_GetStrongTabImpl(0);
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
VERIFY_ARE_EQUAL(1, tab->GetLeafPaneCount());
});
VERIFY_SUCCEEDED(result);
Expand All @@ -479,7 +479,7 @@ namespace TerminalAppLocalTests
page->_SplitPane(SplitState::Automatic, SplitType::Duplicate, nullptr);

VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
auto tab = page->_GetStrongTabImpl(0);
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
VERIFY_ARE_EQUAL(2, tab->GetLeafPaneCount());
});
VERIFY_SUCCEEDED(result);
Expand All @@ -497,7 +497,7 @@ namespace TerminalAppLocalTests
page->_SplitPane(SplitState::Automatic, SplitType::Duplicate, nullptr);

VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
auto tab = page->_GetStrongTabImpl(0);
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
VERIFY_ARE_EQUAL(2,
tab->GetLeafPaneCount(),
L"We should gracefully do nothing here - the profile no longer exists.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@

<!-- If you don't reference these projects here, the
_ConsoleGenerateAdditionalWinmdManifests step won't gather the winmd's -->
<ProjectReference Include="$(OpenConsoleDir)src\cascadia\TerminalSettingsEditor\Microsoft.Terminal.Settings.Editor.vcxproj" />
<ProjectReference Include="$(OpenConsoleDir)src\cascadia\TerminalControl\TerminalControl.vcxproj" />
<ProjectReference Include="$(OpenConsoleDir)src\cascadia\TerminalConnection\TerminalConnection.vcxproj" />
<ProjectReference Include="$(OpenConsoleDir)src\cascadia\TerminalApp\dll\TerminalApp.vcxproj" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@
<ProjectReference Include="$(OpenConsoleDir)src\cascadia\TerminalConnection\TerminalConnection.vcxproj">
<Project>{CA5CAD1A-C46D-4588-B1C0-40F31AE9100B}</Project>
</ProjectReference>

<ProjectReference Include="$(OpenConsoleDir)src\cascadia\TerminalSettingsEditor\Microsoft.Terminal.Settings.Editor.vcxproj" />
<ProjectReference Include="$(OpenConsoleDir)src\cascadia\TerminalControl\TerminalControl.vcxproj" />

<ProjectReference Include="$(OpenConsoleDir)src\cascadia\TerminalApp\dll\TerminalApp.vcxproj">
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/ActionArgs.idl
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ namespace TerminalApp
{
SettingsFile = 0,
DefaultsFile,
AllFiles
AllFiles,
SettingsUI
};

[default_interface] runtimeclass NewTerminalArgs {
Expand Down
90 changes: 52 additions & 38 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,22 +121,27 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleTogglePaneZoom(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
{
auto activeTab = _GetFocusedTab();

// Don't do anything if there's only one pane. It's already zoomed.
if (activeTab && activeTab->GetLeafPaneCount() > 1)
if (auto focusedTab = _GetFocusedTab())
{
// First thing's first, remove the current content from the UI
// tree. This is important, because we might be leaving zoom, and if
// a pane is zoomed, then it's currently in the UI tree, and should
// be removed before it's re-added in Pane::Restore
_tabContent.Children().Clear();
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
{
// Don't do anything if there's only one pane. It's already zoomed.
if (activeTab && activeTab->GetLeafPaneCount() > 1)
{
// First thing's first, remove the current content from the UI
// tree. This is important, because we might be leaving zoom, and if
// a pane is zoomed, then it's currently in the UI tree, and should
// be removed before it's re-added in Pane::Restore
_tabContent.Children().Clear();

activeTab->ToggleZoom();
activeTab->ToggleZoom();

// Update the selected tab, to trigger us to re-add the tab's GetRootElement to the UI tree
_UpdatedSelectedTab(_tabView.SelectedIndex());
// Update the selected tab, to trigger us to re-add the tab's tab content to the UI tree
_UpdatedSelectedTab(_tabView.SelectedIndex());
}
}
}

args.Handled(true);
}

Expand Down Expand Up @@ -315,16 +320,19 @@ namespace winrt::TerminalApp::implementation
args.Handled(false);
if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::SetColorSchemeArgs>())
{
if (auto activeTab = _GetFocusedTab())
if (auto focusedTab = _GetFocusedTab())
{
if (auto activeControl = activeTab->GetActiveTerminalControl())
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
{
if (const auto scheme = _settings.GlobalSettings().ColorSchemes().TryLookup(realArgs.SchemeName()))
if (auto activeControl = activeTab->GetActiveTerminalControl())
{
auto controlSettings = activeControl.Settings().as<TerminalSettings>();
controlSettings->ApplyColorScheme(scheme);
activeControl.UpdateSettings(*controlSettings);
args.Handled(true);
if (const auto scheme = _settings.GlobalSettings().ColorSchemes().TryLookup(realArgs.SchemeName()))
{
auto controlSettings = activeControl.Settings().as<TerminalSettings>();
controlSettings->ApplyColorScheme(scheme);
activeControl.UpdateSettings(*controlSettings);
args.Handled(true);
}
}
}
}
Expand All @@ -344,16 +352,18 @@ namespace winrt::TerminalApp::implementation
}
}

auto activeTab = _GetFocusedTab();
if (activeTab)
if (auto focusedTab = _GetFocusedTab())
{
if (tabColor.has_value())
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
{
activeTab->SetRuntimeTabColor(tabColor.value());
}
else
{
activeTab->ResetRuntimeTabColor();
if (tabColor.has_value())
{
activeTab->SetRuntimeTabColor(tabColor.value());
}
else
{
activeTab->ResetRuntimeTabColor();
}
}
}
args.Handled(true);
Expand All @@ -362,10 +372,12 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleOpenTabColorPicker(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
{
auto activeTab = _GetFocusedTab();
if (activeTab)
if (auto focusedTab = _GetFocusedTab())
{
activeTab->ActivateColorPicker();
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
{
activeTab->ActivateColorPicker();
}
}
args.Handled(true);
}
Expand All @@ -380,16 +392,18 @@ namespace winrt::TerminalApp::implementation
title = realArgs.Title();
}

auto activeTab = _GetFocusedTab();
if (activeTab)
if (auto focusedTab = _GetFocusedTab())
{
if (title.has_value())
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
{
activeTab->SetTabText(title.value());
}
else
{
activeTab->ResetTabText();
if (title.has_value())
{
activeTab->SetTabText(title.value());
}
else
{
activeTab->ResetTabText();
}
}
}
args.Handled(true);
Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
#pragma once

#include "AppLogic.g.h"

#include "Tab.h"
#include "CascadiaSettings.h"
#include "TerminalPage.h"
#include "Jumplist.h"
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/CommandPalette.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,7 @@ namespace winrt::TerminalApp::implementation
// - <none>
void CommandPalette::OnTabsChanged(const IInspectable& s, const IVectorChangedEventArgs& e)
{
if (auto tabList = s.try_as<IObservableVector<TerminalApp::Tab>>())
if (auto tabList = s.try_as<IObservableVector<TerminalApp::ITab>>())
{
auto idx = e.Index();
auto changedEvent = e.CollectionChange();
Expand Down
20 changes: 20 additions & 0 deletions src/cascadia/TerminalApp/ITab.idl
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import "Command.idl";

namespace TerminalApp
{
interface ITab
{
String Title { get; };
Windows.UI.Xaml.Controls.IconSource IconSource { get; };
Command SwitchToTabCommand;
Microsoft.UI.Xaml.Controls.TabViewItem TabViewItem { get; };
Windows.UI.Xaml.FrameworkElement Content { get; };
Windows.UI.Xaml.FocusState FocusState { get; };

void Focus(Windows.UI.Xaml.FocusState focusState);
void Shutdown();
}
}
Loading

0 comments on commit bde5d5e

Please sign in to comment.