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.

Sign up for GitHub

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

Jump to bottom

Fix SwitchToTab build break #8545

Merged
merged 1 commit into from
Dec 10, 2020
Merged

Fix SwitchToTab build break #8545

merged 1 commit into from
Dec 10, 2020

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Dec 10, 2020 •
edited by DHowett
Loading

#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

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

@ghost ghost added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. labels Dec 10, 2020
@DHowett
Copy link
Member

DHowett commented Dec 10, 2020

The new SA failures are due to VS 16.8.
I am comfortable with an admin override for now.

@DHowett DHowett merged commit d676103 into feature/settings-ui Dec 10, 2020
@DHowett DHowett deleted the dev/cazamor/sui/bugfix-switch-to-tab branch December 10, 2020 18:42
@Don-Vito
Copy link
Contributor

The fix is perfect! Please let me know if I can help somehow 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DHowett DHowett DHowett approved these changes

@zadjii-msft zadjii-msft zadjii-msft approved these changes

Assignees
No one assigned
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@carlos-zamora @DHowett @Don-Vito @zadjii-msft