Skip to content
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

[DRAFT] Make tab header a custom control #8225

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/actions/spell-check/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2277,7 +2277,6 @@ tcome
tcommandline
tcommands
tcon
TDelegated
TDP
TEAMPROJECT
tearoff
Expand Down
2 changes: 1 addition & 1 deletion custom.props
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<XesUseOneStoreVersioning>true</XesUseOneStoreVersioning>
<XesBaseYearForStoreVersion>2020</XesBaseYearForStoreVersion>
<VersionMajor>1</VersionMajor>
<VersionMinor>6</VersionMinor>
<VersionMinor>5</VersionMinor>
<VersionInfoProductName>Windows Terminal</VersionInfoProductName>
</PropertyGroup>
</Project>
17 changes: 8 additions & 9 deletions doc/cascadia/Json-Utility-API.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,19 @@ return a JSON value coerced into the specified type.
When reading into existing storage, it returns a boolean indicating whether that storage was modified.

If the JSON value cannot be converted to the specified type, an exception will be generated.
For non-nullable type conversions (most POD types), `null` is considered to be an invalid type.

```c++
std::string one;
std::optional<std::string> two;

JsonUtils::GetValue(json, one);
// one is populated or an exception is thrown.
// one is populated or unchanged.

JsonUtils::GetValue(json, two);
// two is populated, nullopt or an exception is thrown
// two is populated, nullopt or unchanged

auto three = JsonUtils::GetValue<std::string>(json);
// three is populated or an exception is thrown
// three is populated or zero-initialized

auto four = JsonUtils::GetValue<std::optional<std::string>>(json);
// four is populated or nullopt
Expand Down Expand Up @@ -226,14 +225,14 @@ auto v = JsonUtils::GetValue<int>(json, conv);

-|json type invalid|json null|valid
-|-|-|-
`T`|❌ exception|❌ exception|✔ converted
`T`|❌ exception|🔵 unchanged|✔ converted
`std::optional<T>`|❌ exception|🟨 `nullopt`|✔ converted

### GetValue&lt;T&gt;() (returning)

-|json type invalid|json null|valid
-|-|-|-
`T`|❌ exception|❌ exception|✔ converted
`T`|❌ exception|🟨 `T{}` (zero value)|✔ converted
`std::optional<T>`|❌ exception|🟨 `nullopt`|✔ converted

### GetValueForKey(T&) (type-deducing)
Expand All @@ -243,14 +242,14 @@ a "key not found" state. The remaining three cases are the same.

val type|key not found|_json type invalid_|_json null_|_valid_
-|-|-|-|-
`T`|🔵 unchanged|_❌ exception_|_❌ exception_|_✔ converted_
`std::optional<T>`|🔵 unchanged|_❌ exception_|_🟨 `nullopt`_|_✔ converted_
`T`|🔵 unchanged|_❌ exception_|_🔵 unchanged_|_✔ converted_
`std::optional<T>`|_🔵 unchanged_|_❌ exception_|_🟨 `nullopt`_|_✔ converted_

### GetValueForKey&lt;T&gt;() (return value)

val type|key not found|_json type invalid_|_json null_|_valid_
-|-|-|-|-
`T`|🟨 `T{}` (zero value)|_❌ exception_|_❌ exception_|_✔ converted_
`T`|🟨 `T{}` (zero value)|_❌ exception_|_🟨 `T{}` (zero value)_|_✔ converted_
`std::optional<T>`|🟨 `nullopt`|_❌ exception_|_🟨 `nullopt`_|_✔ converted_

### Future Direction
Expand Down
11 changes: 1 addition & 10 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -753,16 +753,7 @@
},
"backgroundImage": {
"description": "Sets the file location of the image to draw over the window background.",
"oneOf": [
{
"type": ["string", null]
},
{
"enum": [
"desktopWallpaper"
]
}
]
"type": ["string", "null"]
},
"backgroundImageAlignment": {
"default": "center",
Expand Down
13 changes: 12 additions & 1 deletion src/cascadia/LocalTests_SettingsModel/CommandTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ namespace SettingsModelLocalTests
// of from keybindings.

const std::string commands0String{ R"([
{ "name": "command0", "command": { "action": "splitPane", "split": null } },
{ "name": "command1", "command": { "action": "splitPane", "split": "vertical" } },
{ "name": "command2", "command": { "action": "splitPane", "split": "horizontal" } },
{ "name": "command4", "command": { "action": "splitPane" } },
Expand All @@ -156,8 +157,18 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(0u, commands.Size());
auto warnings = implementation::Command::LayerJson(commands, commands0Json);
VERIFY_ARE_EQUAL(0u, warnings.size());
VERIFY_ARE_EQUAL(4u, commands.Size());
VERIFY_ARE_EQUAL(5u, commands.Size());

{
auto command = commands.Lookup(L"command0");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.Action());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action());
const auto& realArgs = command.Action().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
}
{
auto command = commands.Lookup(L"command1");
VERIFY_IS_NOT_NULL(command);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1410,14 +1410,14 @@ namespace SettingsModelLocalTests
}
void DeserializationTests::TestProfileBackgroundImageWithDesktopWallpaper()
{
const winrt::hstring expectedBackgroundImagePath{ L"desktopWallpaper" };
const winrt::hstring expectedBackgroundImagePath{ winrt::to_hstring("DesktopWallpaper") };

const std::string settingsJson{ R"(
{
"profiles": [
{
"name": "profile0",
"backgroundImage": "desktopWallpaper"
"backgroundImage": "DesktopWallpaper"
}
]
})" };
Expand Down
12 changes: 11 additions & 1 deletion src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ namespace SettingsModelLocalTests
void KeyBindingsTests::TestSplitPaneArgs()
{
const std::string bindings0String{ R"([
{ "keys": ["ctrl+c"], "command": { "action": "splitPane", "split": null } },
{ "keys": ["ctrl+d"], "command": { "action": "splitPane", "split": "vertical" } },
{ "keys": ["ctrl+e"], "command": { "action": "splitPane", "split": "horizontal" } },
{ "keys": ["ctrl+g"], "command": { "action": "splitPane" } },
Expand All @@ -334,8 +335,17 @@ namespace SettingsModelLocalTests
VERIFY_IS_NOT_NULL(keymap);
VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size());
keymap->LayerJson(bindings0Json);
VERIFY_ARE_EQUAL(4u, keymap->_keyShortcuts.size());
VERIFY_ARE_EQUAL(5u, keymap->_keyShortcuts.size());

{
KeyChord kc{ true, false, false, static_cast<int32_t>('C') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('D') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
Expand Down
63 changes: 0 additions & 63 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "../TerminalApp/TabRowControl.h"
#include "../TerminalApp/ShortcutActionDispatch.h"
#include "../TerminalApp/TerminalTab.h"
#include "../TerminalApp/CommandPalette.h"
#include "../CppWinrtTailored.h"

using namespace Microsoft::Console;
Expand Down Expand Up @@ -83,7 +82,6 @@ namespace TerminalAppLocalTests
TEST_METHOD(CloseZoomedPane);

TEST_METHOD(NextMRUTab);
TEST_METHOD(VerifyCommandPaletteTabSwitcherOrder);

TEST_CLASS_SETUP(ClassSetup)
{
Expand Down Expand Up @@ -850,65 +848,4 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(3u, focusedIndex, L"Verify the fourth tab is the focused one");
});
}

void TabTests::VerifyCommandPaletteTabSwitcherOrder()
{
// This is a test for GH#8188 - we want to make sure that the order of tabs
// is preserved in the CommandPalette's TabSwitcher

auto page = _commonSetup();

Log::Comment(L"Create 3 additional tabs");
RunOnUIThread([&page]() {
NewTerminalArgs newTerminalArgs{ 1 };
page->_OpenNewTab(newTerminalArgs);
page->_OpenNewTab(newTerminalArgs);
page->_OpenNewTab(newTerminalArgs);
});
VERIFY_ARE_EQUAL(4u, page->_mruTabActions.Size());

Log::Comment(L"give alphabetical names to all switch tab actions");
RunOnUIThread([&page]() {
page->_tabs.GetAt(0).SwitchToTabCommand().Name(L"a");
page->_tabs.GetAt(1).SwitchToTabCommand().Name(L"b");
page->_tabs.GetAt(2).SwitchToTabCommand().Name(L"c");
page->_tabs.GetAt(3).SwitchToTabCommand().Name(L"d");
});

Log::Comment(L"Change the tab switch order to MRU switching");
TestOnUIThread([&page]() {
page->_settings.GlobalSettings().TabSwitcherMode(TabSwitcherMode::MostRecentlyUsed);
});

Log::Comment(L"Select the tabs from 0 to 3");
RunOnUIThread([&page]() {
page->_UpdatedSelectedTab(0);
page->_UpdatedSelectedTab(1);
page->_UpdatedSelectedTab(2);
page->_UpdatedSelectedTab(3);
});

VERIFY_ARE_EQUAL(4u, page->_mruTabActions.Size());
VERIFY_ARE_EQUAL(L"d", page->_mruTabActions.GetAt(0).Name());
VERIFY_ARE_EQUAL(L"c", page->_mruTabActions.GetAt(1).Name());
VERIFY_ARE_EQUAL(L"b", page->_mruTabActions.GetAt(2).Name());
VERIFY_ARE_EQUAL(L"a", page->_mruTabActions.GetAt(3).Name());

Log::Comment(L"Switch to the next MRU tab, which is the third tab");
RunOnUIThread([&page]() {
page->_SelectNextTab(true);
});

const auto palette = winrt::get_self<implementation::CommandPalette>(page->CommandPalette());

VERIFY_ARE_EQUAL(1u, palette->_switcherStartIdx, L"Verify the index is 1 as we went right");
VERIFY_ARE_EQUAL(implementation::CommandPaletteMode::TabSwitchMode, palette->_currentMode, L"Verify we are in the tab switcher mode");

Log::Comment(L"Verify command palette preserves MRU order of tabs");
VERIFY_ARE_EQUAL(4u, palette->_filteredActions.Size());
VERIFY_ARE_EQUAL(L"d", palette->_filteredActions.GetAt(0).Command().Name());
VERIFY_ARE_EQUAL(L"c", palette->_filteredActions.GetAt(1).Command().Name());
VERIFY_ARE_EQUAL(L"b", palette->_filteredActions.GetAt(2).Command().Name());
VERIFY_ARE_EQUAL(L"a", palette->_filteredActions.GetAt(3).Command().Name());
}
}
Loading