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

Lazy load CommandPalette and AboutDialog #15203

Merged
merged 2 commits into from
Apr 19, 2023
Merged
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
6 changes: 3 additions & 3 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,7 @@ namespace TerminalAppLocalTests
// If you don't do this, the palette will just stay open, and the
// next time we call _HandleNextTab, we'll continue traversing the
// MRU list, instead of just hoping one entry.
page->CommandPalette().Visibility(Visibility::Collapsed);
page->LoadCommandPalette().Visibility(Visibility::Collapsed);
});

TestOnUIThread([&page]() {
Expand All @@ -1123,7 +1123,7 @@ namespace TerminalAppLocalTests
// If you don't do this, the palette will just stay open, and the
// next time we call _HandleNextTab, we'll continue traversing the
// MRU list, instead of just hoping one entry.
page->CommandPalette().Visibility(Visibility::Collapsed);
page->LoadCommandPalette().Visibility(Visibility::Collapsed);
});

TestOnUIThread([&page]() {
Expand Down Expand Up @@ -1239,7 +1239,7 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(L"a", page->_mruTabs.GetAt(3).Title());
});

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

VERIFY_ARE_EQUAL(winrt::TerminalApp::implementation::CommandPaletteMode::TabSwitchMode, palette->_currentMode, L"Verify we are in the tab switcher mode");
// At this point, the contents of the command palette's _mruTabs list is
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/AboutDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace winrt::TerminalApp::implementation
AboutDialog::AboutDialog()
{
InitializeComponent();
_queueUpdateCheck();
}

winrt::hstring AboutDialog::ApplicationDisplayName()
Expand Down Expand Up @@ -74,7 +75,7 @@ namespace winrt::TerminalApp::implementation
_PropertyChangedHandlers(*this, WUX::Data::PropertyChangedEventArgs{ L"UpdatesAvailable" });
}

winrt::fire_and_forget AboutDialog::QueueUpdateCheck()
winrt::fire_and_forget AboutDialog::_queueUpdateCheck()
{
auto strongThis = get_strong();
auto now{ std::chrono::system_clock::now() };
Expand Down
5 changes: 2 additions & 3 deletions src/cascadia/TerminalApp/AboutDialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,20 @@ namespace winrt::TerminalApp::implementation
winrt::hstring ApplicationVersion();
bool UpdatesAvailable() const;
winrt::hstring PendingUpdateVersion() const;
winrt::fire_and_forget QueueUpdateCheck();

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
WINRT_OBSERVABLE_PROPERTY(bool, CheckingForUpdates, _PropertyChangedHandlers, false);

private:
friend struct AboutDialogT<AboutDialog>; // for Xaml to bind events

void _SetPendingUpdateVersion(const winrt::hstring& pendingUpdateVersion);

std::chrono::system_clock::time_point _lastUpdateCheck{};
winrt::hstring _pendingUpdateVersion;

void _SetPendingUpdateVersion(const winrt::hstring& pendingUpdateVersion);
void _ThirdPartyNoticesOnClick(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs);
void _SendFeedbackOnClick(const IInspectable& sender, const Windows::UI::Xaml::Controls::ContentDialogButtonClickEventArgs& eventArgs);
winrt::fire_and_forget _queueUpdateCheck();
};
}

Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/TerminalApp/AboutDialog.idl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,5 @@ namespace TerminalApp
Boolean CheckingForUpdates { get; };
Boolean UpdatesAvailable { get; };
String PendingUpdateVersion { get; };

void QueueUpdateCheck();
}
}
15 changes: 8 additions & 7 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -613,10 +613,10 @@ namespace winrt::TerminalApp::implementation
{
if (const auto& realArgs = args.ActionArgs().try_as<ToggleCommandPaletteArgs>())
{
CommandPalette().EnableCommandPaletteMode(realArgs.LaunchMode());
CommandPalette().Visibility(CommandPalette().Visibility() == Visibility::Visible ?
Visibility::Collapsed :
Visibility::Visible);
const auto p = LoadCommandPalette();
const auto v = p.Visibility() == Visibility::Visible ? Visibility::Collapsed : Visibility::Visible;
p.EnableCommandPaletteMode(realArgs.LaunchMode());
p.Visibility(v);
args.Handled(true);
}
}
Expand Down Expand Up @@ -799,9 +799,10 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleTabSearch(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
CommandPalette().SetTabs(_tabs, _mruTabs);
CommandPalette().EnableTabSearchMode();
CommandPalette().Visibility(Visibility::Visible);
const auto p = LoadCommandPalette();
p.SetTabs(_tabs, _mruTabs);
p.EnableTabSearchMode();
p.Visibility(Visibility::Visible);

args.Handled(true);
}
Expand Down
14 changes: 8 additions & 6 deletions src/cascadia/TerminalApp/TabManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -600,13 +600,14 @@ namespace winrt::TerminalApp::implementation
}
else
{
CommandPalette().SetTabs(_tabs, _mruTabs);
const auto p = LoadCommandPalette();
p.SetTabs(_tabs, _mruTabs);

// Otherwise, set up the tab switcher in the selected mode, with
// the given ordering, and make it visible.
CommandPalette().EnableTabSwitcherMode(index, tabSwitchMode);
CommandPalette().Visibility(Visibility::Visible);
CommandPalette().SelectNextItem(bMoveRight);
p.EnableTabSwitcherMode(index, tabSwitchMode);
p.Visibility(Visibility::Visible);
p.SelectNextItem(bMoveRight);
}
}

Expand Down Expand Up @@ -916,7 +917,7 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_UpdatedSelectedTab(const winrt::TerminalApp::TabBase& tab)
{
// Unfocus all the tabs.
for (auto tab : _tabs)
for (const auto& tab : _tabs)
{
tab.Focus(FocusState::Unfocused);
}
Expand All @@ -936,7 +937,8 @@ namespace winrt::TerminalApp::implementation
// When the tab switcher is eventually dismissed, the focus will
// get tossed back to the focused terminal control, so we don't
// need to worry about focus getting lost.
if (CommandPalette().Visibility() != Visibility::Visible)
const auto p = CommandPaletteElement();
if (!p || p.Visibility() != Visibility::Visible)
{
tab.Focus(FocusState::Programmatic);
_UpdateMRUTab(tab);
Expand Down
89 changes: 47 additions & 42 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,14 @@ namespace winrt::TerminalApp::implementation

_settings = settings;

// Make sure to _UpdateCommandsForPalette before
// _RefreshUIForSettingsReload. _UpdateCommandsForPalette will make
// sure the KeyChordText of Commands is updated, which needs to
// happen before the Settings UI is reloaded and tries to re-read
// those values.
_UpdateCommandsForPalette();
CommandPalette().SetActionMap(_settings.ActionMap());
// Make sure to call SetCommands before _RefreshUIForSettingsReload.
// SetCommands will make sure the KeyChordText of Commands is updated, which needs
// to happen before the Settings UI is reloaded and tries to re-read those values.
if (const auto p = CommandPaletteElement())
{
p.SetCommands(_settings.GlobalSettings().ActionMap().ExpandedCommands());
p.SetActionMap(_settings.ActionMap());
}

if (needRefreshUI)
{
Expand Down Expand Up @@ -255,20 +256,6 @@ namespace winrt::TerminalApp::implementation

_UpdateTabWidthMode();

// When the visibility of the command palette changes to "collapsed",
// the palette has been closed. Toss focus back to the currently active
// control.
CommandPalette().RegisterPropertyChangedCallback(UIElement::VisibilityProperty(), [this](auto&&, auto&&) {
if (CommandPalette().Visibility() == Visibility::Collapsed)
{
_FocusActiveControl(nullptr, nullptr);
}
});
CommandPalette().DispatchCommandRequested({ this, &TerminalPage::_OnDispatchCommandRequested });
CommandPalette().CommandLineExecutionRequested({ this, &TerminalPage::_OnCommandLineExecutionRequested });
CommandPalette().SwitchToTabRequested({ this, &TerminalPage::_OnSwitchToTabRequested });
CommandPalette().PreviewAction({ this, &TerminalPage::_PreviewActionHandler });

// Settings AllowDependentAnimations will affect whether animations are
// enabled application-wide, so we don't need to check it each time we
// want to create an animation.
Expand Down Expand Up @@ -684,7 +671,6 @@ namespace winrt::TerminalApp::implementation
// Notes link, send feedback link and privacy policy link.
void TerminalPage::_ShowAboutDialog()
{
AboutDialog().QueueUpdateCheck();
_ShowDialogHelper(L"AboutDialog");
}

Expand Down Expand Up @@ -1333,8 +1319,9 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_CommandPaletteButtonOnClick(const IInspectable&,
const RoutedEventArgs&)
{
CommandPalette().EnableCommandPaletteMode(CommandPaletteLaunchMode::Action);
CommandPalette().Visibility(Visibility::Visible);
auto p = LoadCommandPalette();
p.EnableCommandPaletteMode(CommandPaletteLaunchMode::Action);
p.Visibility(Visibility::Visible);
}

// Method Description:
Expand All @@ -1350,7 +1337,7 @@ namespace winrt::TerminalApp::implementation
}

// Method Description:
// - Called when the users pressed keyBindings while CommandPalette is open.
// - Called when the users pressed keyBindings while CommandPaletteElement is open.
// - As of GH#8480, this is also bound to the TabRowControl's KeyUp event.
// That should only fire when focus is in the tab row, which is hard to
// do. Notably, that's possible:
Expand Down Expand Up @@ -1421,7 +1408,7 @@ namespace winrt::TerminalApp::implementation
return;
}

if (const auto p = CommandPalette(); p.Visibility() == Visibility::Visible && cmd.ActionAndArgs().Action() != ShortcutAction::ToggleCommandPalette)
if (const auto p = CommandPaletteElement(); p.Visibility() == Visibility::Visible && cmd.ActionAndArgs().Action() != ShortcutAction::ToggleCommandPalette)
{
p.Visibility(Visibility::Collapsed);
}
Expand Down Expand Up @@ -1745,6 +1732,40 @@ namespace winrt::TerminalApp::implementation
}
return nullptr;
}

CommandPalette TerminalPage::LoadCommandPalette()
{
if (const auto p = CommandPaletteElement())
{
return p;
}

return _loadCommandPaletteSlowPath();
}

CommandPalette TerminalPage::_loadCommandPaletteSlowPath()
{
const auto p = FindName(L"CommandPaletteElement").as<CommandPalette>();

p.SetCommands(_settings.GlobalSettings().ActionMap().ExpandedCommands());
p.SetActionMap(_settings.ActionMap());

// When the visibility of the command palette changes to "collapsed",
// the palette has been closed. Toss focus back to the currently active control.
p.RegisterPropertyChangedCallback(UIElement::VisibilityProperty(), [this](auto&&, auto&&) {
if (CommandPaletteElement().Visibility() == Visibility::Collapsed)
{
_FocusActiveControl(nullptr, nullptr);
}
});
p.DispatchCommandRequested({ this, &TerminalPage::_OnDispatchCommandRequested });
p.CommandLineExecutionRequested({ this, &TerminalPage::_OnCommandLineExecutionRequested });
p.SwitchToTabRequested({ this, &TerminalPage::_OnSwitchToTabRequested });
p.PreviewAction({ this, &TerminalPage::_PreviewActionHandler });

return p;
}

// Method Description:
// - Warn the user that they are about to close all open windows, then
// signal that we want to close everything.
Expand Down Expand Up @@ -3140,21 +3161,6 @@ namespace winrt::TerminalApp::implementation
}
}

// Method Description:
// - Repopulates the list of commands in the command palette with the
// current commands in the settings. Also updates the keybinding labels to
// reflect any matching keybindings.
// Arguments:
// - <none>
// Return Value:
// - <none>
void TerminalPage::_UpdateCommandsForPalette()
{
// Update the command palette when settings reload
const auto& expanded{ _settings.GlobalSettings().ActionMap().ExpandedCommands() };
CommandPalette().SetCommands(expanded);
}

// Method Description:
// - Sets the initial actions to process on startup. We'll make a copy of
// this list, and process these actions when we're loaded.
Expand Down Expand Up @@ -4828,5 +4834,4 @@ namespace winrt::TerminalApp::implementation
// _RemoveTab will make sure to null out the _stashed.draggedTab
_RemoveTab(*_stashed.draggedTab);
}

}
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ namespace winrt::TerminalApp::implementation
winrt::hstring ApplicationDisplayName();
winrt::hstring ApplicationVersion();

CommandPalette LoadCommandPalette();
winrt::fire_and_forget RequestQuit();
winrt::fire_and_forget CloseWindow(bool bypassDialog);

Expand Down Expand Up @@ -274,6 +275,7 @@ namespace winrt::TerminalApp::implementation

winrt::Microsoft::Terminal::TerminalConnection::ConptyConnection::NewConnection_revoker _newConnectionRevoker;

__declspec(noinline) CommandPalette _loadCommandPaletteSlowPath();
winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowDialogHelper(const std::wstring_view& name);

void _ShowAboutDialog();
Expand Down Expand Up @@ -312,7 +314,6 @@ namespace winrt::TerminalApp::implementation
void _UpdateTabIcon(TerminalTab& tab);
void _UpdateTabView();
void _UpdateTabWidthMode();
void _UpdateCommandsForPalette();
void _SetBackgroundImage(const winrt::Microsoft::Terminal::Settings::Model::IAppearanceConfig& newAppearance);

void _DuplicateFocusedTab();
Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@
-->

<local:AboutDialog x:Name="AboutDialog"
Grid.Row="2" />
Grid.Row="2"
x:Load="False" />

<ContentDialog x:Name="QuitDialog"
x:Uid="QuitDialog"
Expand Down Expand Up @@ -167,9 +168,10 @@
</TextBlock>
</ContentDialog>

<local:CommandPalette x:Name="CommandPalette"
<local:CommandPalette x:Name="CommandPaletteElement"
Grid.Row="2"
VerticalAlignment="Stretch"
x:Load="False"
PreviewKeyDown="_KeyDownHandler"
Visibility="Collapsed" />

Expand Down