Skip to content

Commit

Permalink
Fix SwitchToTab build break (#8545)
Browse files Browse the repository at this point in the history
#8420 removed `SwitchToTab()` as a responsibility of `TabBase` and replaces `_mruTabActions` with `_mruTabs` (conceptually). This PR fixes the build break by...
- replacing `TerminalPage`'s reference to the SettingsTab's SwitchToTab command, with a reference to the tab itself
- using that reference to maintain existing tab switching behavior

## References
#1564 - Settings UI
#8420 - Command Palette + SwitchToTab refactoring

## PR Checklist
* [X] Closes #8538

## Validation Steps Performed
✅ Open SUI --> switch to a different tab --> try opening SUI again --> switches to existing SUI
✅ Open SUI --> switch to a different tab --> reorder tabs --> try opening SUI again --> switches to existing SUI
  • Loading branch information
carlos-zamora authored Dec 10, 2020
1 parent 69693f6 commit d676103
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 10 deletions.
3 changes: 0 additions & 3 deletions src/cascadia/TerminalApp/SettingsTab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,6 @@ namespace winrt::TerminalApp::implementation
// The TabViewItem Icon needs MUX while the IconSourceElement in the CommandPalette needs WUX...
Icon(glyph);
TabViewItem().IconSource(IconPathConverter::IconSourceMUX(glyph));

// Update SwitchToTab command's icon
SwitchToTabCommand().Icon(glyph);
}
}
}
11 changes: 5 additions & 6 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2793,7 +2793,7 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_OpenSettingsUI()
{
// If we're holding the settings tab's switch command, don't create a new one, switch to the existing one.
if (!_switchToSettingsCommand)
if (!_settingsTab)
{
winrt::Microsoft::Terminal::Settings::Editor::MainPage sui{ _settings };
if (_hostingHwnd)
Expand All @@ -2809,11 +2809,10 @@ namespace winrt::TerminalApp::implementation
});

auto newTabImpl = winrt::make_self<SettingsTab>(sui);
_MakeSwitchToTabCommand(*newTabImpl, _tabs.Size());

// Add the new tab to the list of our tabs.
_tabs.Append(*newTabImpl);
_mruTabActions.Append(newTabImpl->SwitchToTabCommand());
_mruTabs.Append(*newTabImpl);

newTabImpl->SetDispatch(*_actionDispatch);

Expand All @@ -2833,20 +2832,20 @@ namespace winrt::TerminalApp::implementation
newTabImpl->Closed([tabViewItem, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
if (auto page{ weakThis.get() })
{
page->_switchToSettingsCommand = nullptr;
page->_settingsTab = nullptr;
page->_RemoveOnCloseRoutine(tabViewItem, page);
}
});

_switchToSettingsCommand = newTabImpl->SwitchToTabCommand();
_settingsTab = *newTabImpl;

// This kicks off TabView::SelectionChanged, in response to which
// we'll attach the terminal's Xaml control to the Xaml root.
_tabView.SelectedItem(tabViewItem);
}
else
{
_actionDispatch->DoAction(_switchToSettingsCommand.Action());
_tabView.SelectedItem(_settingsTab.TabViewItem());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ namespace winrt::TerminalApp::implementation

void _UpdateTabIndices();

winrt::Microsoft::Terminal::Settings::Model::Command _switchToSettingsCommand{ nullptr };
TerminalApp::SettingsTab _settingsTab{ nullptr };

bool _isInFocusMode{ false };
bool _isFullscreen{ false };
Expand Down

0 comments on commit d676103

Please sign in to comment.