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

Split AppLogic into "App logic" and "Window logic" #14825

Merged
merged 28 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
936c01f
Start splitting AppLogic into AppLogic and Window logic
zadjii-msft Jan 30, 2023
439b21f
this is dangerously close to compiling
zadjii-msft Jan 30, 2023
99bc280
It doesn't crash on launch. That's something. There's no startupActio…
zadjii-msft Jan 31, 2023
2195515
it launches
zadjii-msft Jan 31, 2023
5116ca1
I think the todo's that are left, we can move on without them for now.
zadjii-msft Jan 31, 2023
cfa6108
Merge remote-tracking branch 'origin/main' into dev/migrie/oop/3/fore…
zadjii-msft Feb 10, 2023
e40575b
let's do it
zadjii-msft Feb 10, 2023
33685d9
bwahahahahaha
zadjii-msft Feb 13, 2023
a4f19a9
spel
zadjii-msft Feb 13, 2023
82224bc
this was usnused
zadjii-msft Feb 13, 2023
d9d4d2e
get the tests to build, at least
zadjii-msft Feb 13, 2023
118bffa
fix the tests
zadjii-msft Feb 14, 2023
273f2a8
Merge remote-tracking branch 'origin/main' into dev/migrie/oop/3/fore…
zadjii-msft Feb 23, 2023
c79f27c
std::vectors are better
zadjii-msft Feb 27, 2023
b589d09
Lots of PR nits
zadjii-msft Feb 28, 2023
0199aba
the last nits, I think
zadjii-msft Feb 28, 2023
ada3f42
well that is an underwhelming codeformat commit. Thanks VS
zadjii-msft Feb 28, 2023
6dead99
PR nits
zadjii-msft Mar 9, 2023
434abc2
Remove the need for TerminalPage to know the number of open windows
zadjii-msft Mar 9, 2023
1b59eb9
save this for later, we should only need ot look it up once
zadjii-msft Mar 9, 2023
39a9450
to wit, also make sure that there is a tablayout
zadjii-msft Mar 9, 2023
44b238e
Redo how ownership of WindowProperties works. This is cleaner by a go…
zadjii-msft Mar 9, 2023
073350e
fix the tests too
zadjii-msft Mar 10, 2023
ca511c9
this is why I don't use VS
zadjii-msft Mar 10, 2023
f70775a
I don't trust you phyllis
zadjii-msft Mar 10, 2023
6aec80b
A hotfix for the selfhost build
zadjii-msft Mar 13, 2023
0808f94
Merge remote-tracking branch 'origin/main' into dev/migrie/oop/3/fore…
zadjii-msft Mar 17, 2023
5b3dc08
last nits, let's do this
zadjii-msft Mar 17, 2023
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
27 changes: 23 additions & 4 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ namespace TerminalAppLocalTests
// an updated TAEF that will let us install framework packages when the test
// package is deployed. Until then, these tests won't deploy in CI.

struct WindowProperties : winrt::implements<WindowProperties, winrt::TerminalApp::IWindowProperties>
{
WINRT_PROPERTY(winrt::hstring, WindowName);
WINRT_PROPERTY(uint64_t, WindowId);
WINRT_PROPERTY(winrt::hstring, WindowNameForDisplay);
WINRT_PROPERTY(winrt::hstring, WindowIdForDisplay);

public:
bool IsQuakeWindow() { return _WindowName == L"_quake"; };
};

class TabTests
{
// For this set of tests, we need to activate some XAML content. For
Expand Down Expand Up @@ -110,6 +121,7 @@ namespace TerminalAppLocalTests
void _initializeTerminalPage(winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage>& page,
CascadiaSettings initialSettings);
winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage> _commonSetup();
winrt::com_ptr<WindowProperties> _windowProperties;
};

template<typename TFunction>
Expand Down Expand Up @@ -239,11 +251,15 @@ namespace TerminalAppLocalTests
// it's weird.
winrt::TerminalApp::TerminalPage projectedPage{ nullptr };

_windowProperties = winrt::make_self<WindowProperties>();
winrt::TerminalApp::IWindowProperties iProps{ *_windowProperties };

Log::Comment(NoThrowString().Format(L"Construct the TerminalPage"));
auto result = RunOnUIThread([&projectedPage, &page, initialSettings]() {
auto result = RunOnUIThread([&projectedPage, &page, initialSettings, iProps]() {
projectedPage = winrt::TerminalApp::TerminalPage();
page.copy_from(winrt::get_self<winrt::TerminalApp::implementation::TerminalPage>(projectedPage));
page->_settings = initialSettings;
page->WindowProperties(iProps);
});
VERIFY_SUCCEEDED(result);

Expand Down Expand Up @@ -1242,10 +1258,13 @@ namespace TerminalAppLocalTests
END_TEST_METHOD_PROPERTIES()

auto page = _commonSetup();
page->RenameWindowRequested([&page](auto&&, const winrt::TerminalApp::RenameWindowRequestedArgs args) {
page->RenameWindowRequested([&page, this](auto&&, const winrt::TerminalApp::RenameWindowRequestedArgs args) {
// In the real terminal, this would bounce up to the monarch and
// come back down. Instead, immediately call back and set the name.
page->WindowName(args.ProposedName());
//
// This replicates how TerminalWindow works
_windowProperties->WindowName(args.ProposedName());
page->WindowNameChanged();
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
});

auto windowNameChanged = false;
Expand All @@ -1260,7 +1279,7 @@ namespace TerminalAppLocalTests
page->_RequestWindowRename(winrt::hstring{ L"Foo" });
});
TestOnUIThread([&]() {
VERIFY_ARE_EQUAL(L"Foo", page->_WindowName);
VERIFY_ARE_EQUAL(L"Foo", page->WindowName());
VERIFY_IS_TRUE(windowNameChanged,
L"The window name should have changed, and we should have raised a notification that WindowNameForDisplay changed");
});
Expand Down
19 changes: 2 additions & 17 deletions src/cascadia/TerminalApp/App.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,22 +77,7 @@ namespace winrt::TerminalApp::implementation
/// <param name="e">Details about the launch request and process.</param>
void App::OnLaunched(const LaunchActivatedEventArgs& /*e*/)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily as a part of this PR, but can we just outright remove this function and all the other pure UWP stuff that's left over?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. It's Real Dead.

{
// if this is a UWP... it means its our problem to hook up the content to the window here.
if (_isUwp)
{
auto content = Window::Current().Content();
if (content == nullptr)
{
auto logic = Logic();
logic.RunAsUwp(); // Must set UWP status first, settings might change based on it.
logic.ReloadSettings();
logic.Create();

auto page = logic.GetRoot().as<TerminalPage>();

Window::Current().Content(page);
Window::Current().Activate();
}
}
// We used to support a pure UWP version of the Terminal. This method
// was only ever used to do UWP-specific setup of our App.
}
}
Loading