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

fix regression with terminal tabs/panel #236500

Closed
wants to merge 1 commit into from
Closed

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Dec 18, 2024

fix #235931

With this change #235931 (comment), we now have to call the view container's focus when it becomes visible

panel.mov

@meganrogge meganrogge requested a review from Tyriar December 18, 2024 17:41
@meganrogge meganrogge self-assigned this Dec 18, 2024
@meganrogge meganrogge added this to the January 2025 milestone Dec 18, 2024
@meganrogge meganrogge enabled auto-merge (squash) December 18, 2024 17:41
@@ -217,6 +217,7 @@ export class TerminalViewPane extends ViewPane {
// defer focusing the panel to the focus() call
// to prevent overriding preserveFocus for extensions
this._terminalGroupService.showPanel(false);
this.focus();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to contradict the comment above. I don't think toggling visibility means it must also focus?

Copy link
Contributor Author

@meganrogge meganrogge Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

focus was previously called by the pane view container

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, see how our focus function only focuses under certain conditions

Screenshot 2024-12-18 at 12 41 28 PM

@meganrogge
Copy link
Contributor Author

Closing because this will cause problems - esp as it relates to our API

@meganrogge meganrogge closed this Dec 18, 2024
auto-merge was automatically disabled December 18, 2024 20:54

Pull request was closed

@meganrogge meganrogge deleted the merogge/terminal-panel branch December 18, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terminal tab titles disappear until switch between tabs
2 participants