Skip to content

Commit

Permalink
Use only one tab color picker for all tabs, delay load it (#13674)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

- We only ever have 1 color picker now, instead of each tab having its own
- `TerminalPage` constructs this color picker (upon first request for it)
- `TerminalPage` attaches the color picker to the tab that requested for it
- `TerminalTab` detaches the color picker when it is done with it, so that `TerminalPage` can attach it to another tab later on

## References
#5907 

## Validation Steps Performed
User-end behaviour is the same
  • Loading branch information
PankajBhojwani authored Aug 5, 2022
1 parent 74cdffe commit ba08dd2
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 22 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ namespace winrt::TerminalApp::implementation
{
if (const auto activeTab{ _GetFocusedTabImpl() })
{
activeTab->ActivateColorPicker();
activeTab->RequestColorPicker();
}
args.Handled(true);
}
Expand Down
14 changes: 14 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1509,6 +1509,20 @@ namespace winrt::TerminalApp::implementation
}
});

hostingTab.ColorPickerRequested([weakTab, weakThis]() {
auto page{ weakThis.get() };
auto tab{ weakTab.get() };
if (page && tab)
{
if (!page->_tabColorPicker)
{
page->_tabColorPicker = winrt::make<ColorPickupFlyout>();
}

tab->AttachColorPicker(page->_tabColorPicker);
}
});

// Add an event handler for when the terminal or tab wants to set a
// progress indicator on the taskbar
hostingTab.TaskbarProgressChanged({ get_weak(), &TerminalPage::_SetTaskbarProgressHandler });
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ namespace winrt::TerminalApp::implementation
TerminalApp::TabRowControl _tabRow{ nullptr };
Windows::UI::Xaml::Controls::Grid _tabContent{ nullptr };
Microsoft::UI::Xaml::Controls::SplitButton _newTabButton{ nullptr };
winrt::TerminalApp::ColorPickupFlyout _tabColorPicker{ nullptr };

Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };

Expand Down
64 changes: 45 additions & 19 deletions src/cascadia/TerminalApp/TerminalTab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,46 @@ namespace winrt::TerminalApp::implementation
}
}

// Method Description:
// - Attaches the given color picker to ourselves
// - Typically will be called after we have sent a request for the color picker
// Arguments:
// - colorPicker: The color picker that we should attach to ourselves
// Return Value:
// - <none>
void TerminalTab::AttachColorPicker(TerminalApp::ColorPickupFlyout& colorPicker)
{
auto weakThis{ get_weak() };

_tabColorPickup = colorPicker;

_colorSelectedToken = _tabColorPickup.ColorSelected([weakThis](auto newTabColor) {
if (auto tab{ weakThis.get() })
{
tab->SetRuntimeTabColor(newTabColor);
}
});

_colorClearedToken = _tabColorPickup.ColorCleared([weakThis]() {
if (auto tab{ weakThis.get() })
{
tab->ResetRuntimeTabColor();
}
});

_pickerClosedToken = _tabColorPickup.Closed([weakThis](auto&&, auto&&) {
if (auto tab{ weakThis.get() })
{
tab->_tabColorPickup.ColorSelected(tab->_colorSelectedToken);
tab->_tabColorPickup.ColorCleared(tab->_colorClearedToken);
tab->_tabColorPickup.Closed(tab->_pickerClosedToken);
tab->_tabColorPickup = nullptr;
}
});

_tabColorPickup.ShowAt(TabViewItem());
}

// Method Description:
// - Find the currently active pane, and then switch the split direction of
// its parent. E.g. switch from Horizontal to Vertical.
Expand Down Expand Up @@ -1184,27 +1224,12 @@ namespace winrt::TerminalApp::implementation
chooseColorMenuItem.Click([weakThis](auto&&, auto&&) {
if (auto tab{ weakThis.get() })
{
tab->ActivateColorPicker();
tab->RequestColorPicker();
}
});
chooseColorMenuItem.Text(RS_(L"TabColorChoose"));
chooseColorMenuItem.Icon(colorPickSymbol);

// Color Picker (it's convenient to have it here)
_tabColorPickup.ColorSelected([weakThis](auto newTabColor) {
if (auto tab{ weakThis.get() })
{
tab->SetRuntimeTabColor(newTabColor);
}
});

_tabColorPickup.ColorCleared([weakThis]() {
if (auto tab{ weakThis.get() })
{
tab->ResetRuntimeTabColor();
}
});

Controls::MenuFlyoutItem renameTabMenuItem;
{
// "Rename Tab"
Expand Down Expand Up @@ -1597,14 +1622,15 @@ namespace winrt::TerminalApp::implementation
}

// Method Description:
// - Display the tab color picker at the location of the TabViewItem for this tab.
// - Send an event to request for the color picker
// - The listener should attach the color picker via AttachColorPicker()
// Arguments:
// - <none>
// Return Value:
// - <none>
void TerminalTab::ActivateColorPicker()
void TerminalTab::RequestColorPicker()
{
_tabColorPickup.ShowAt(TabViewItem());
_ColorPickerRequestedHandlers();
}

// Method Description:
Expand Down
11 changes: 9 additions & 2 deletions src/cascadia/TerminalApp/TerminalTab.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ namespace winrt::TerminalApp::implementation
std::shared_ptr<Pane> DetachPane();
void AttachPane(std::shared_ptr<Pane> pane);

void AttachColorPicker(winrt::TerminalApp::ColorPickupFlyout& colorPicker);

void SplitPane(winrt::Microsoft::Terminal::Settings::Model::SplitDirection splitType,
const float splitSize,
std::shared_ptr<Pane> newPane);
Expand Down Expand Up @@ -74,7 +76,7 @@ namespace winrt::TerminalApp::implementation
void ThemeColor(const winrt::Microsoft::Terminal::Settings::Model::ThemeColor& color);
void SetRuntimeTabColor(const winrt::Windows::UI::Color& color);
void ResetRuntimeTabColor();
void ActivateColorPicker();
void RequestColorPicker();

void UpdateZoom(std::shared_ptr<Pane> newFocus);
void ToggleZoom();
Expand Down Expand Up @@ -105,6 +107,7 @@ namespace winrt::TerminalApp::implementation
WINRT_CALLBACK(SplitTabRequested, winrt::delegate<>);
WINRT_CALLBACK(FindRequested, winrt::delegate<>);
WINRT_CALLBACK(ExportTabRequested, winrt::delegate<>);
WINRT_CALLBACK(ColorPickerRequested, winrt::delegate<>);
TYPED_EVENT(TaskbarProgressChanged, IInspectable, IInspectable);

private:
Expand All @@ -113,12 +116,16 @@ namespace winrt::TerminalApp::implementation
std::shared_ptr<Pane> _zoomedPane{ nullptr };

winrt::hstring _lastIconPath{};
winrt::TerminalApp::ColorPickupFlyout _tabColorPickup{};
std::optional<winrt::Windows::UI::Color> _runtimeTabColor{};
winrt::TerminalApp::TabHeaderControl _headerControl{};
winrt::TerminalApp::TerminalTabStatus _tabStatus{};
winrt::Microsoft::Terminal::Settings::Model::ThemeColor _themeColor{ nullptr };

winrt::TerminalApp::ColorPickupFlyout _tabColorPickup{ nullptr };
winrt::event_token _colorSelectedToken;
winrt::event_token _colorClearedToken;
winrt::event_token _pickerClosedToken;

struct ControlEventTokens
{
winrt::event_token titleToken;
Expand Down

0 comments on commit ba08dd2

Please sign in to comment.