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

[feature/settings-ui] command palette build break on SettingsTab #8538

Closed
DHowett opened this issue Dec 10, 2020 · 0 comments
Closed

[feature/settings-ui] command palette build break on SettingsTab #8538

DHowett opened this issue Dec 10, 2020 · 0 comments
Assignees
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. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Milestone

Comments

@DHowett
Copy link
Member

DHowett commented Dec 10, 2020

PR #8415 #8420 removed SwitchToTabCommand and made the MRU list use tab objects directly. This poses a problem for the way we cache and re-execute the "_switchToSettingsCommand".

We need to retool this. Branch currently doesn't build.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Dec 10, 2020
@DHowett DHowett 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
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Dec 10, 2020
@DHowett DHowett added this to the Terminal v1.6 milestone Dec 10, 2020
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Dec 10, 2020
@DHowett DHowett changed the title [feature/settings-ui] build break on SettingsTab [feature/settings-ui] command palette build break on SettingsTab Dec 10, 2020
@ghost ghost added In-PR This issue has a related PR Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Dec 10, 2020
DHowett pushed a commit that referenced this issue Dec 10, 2020
#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Development

No branches or pull requests

2 participants