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

Terminal tab titles disappear until switch between tabs #235931

Open
ulugbekna opened this issue Dec 12, 2024 · 14 comments · Fixed by #236281
Open

Terminal tab titles disappear until switch between tabs #235931

ulugbekna opened this issue Dec 12, 2024 · 14 comments · Fixed by #236281
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug regression Something that used to work is now broken terminal-tabs
Milestone

Comments

@ulugbekna
Copy link
Contributor

ulugbekna commented Dec 12, 2024

Not sure how to repro, but seems to occur when I hide/unhide Panel visibility

CleanShot.2024-12-12.at.12.09.12.mp4

Version: 1.97.0-insider
Commit: 6a5c8cf
Date: 2024-12-12T05:04:17.220Z
Electron: 32.2.6
ElectronBuildId: 10629634
Chromium: 128.0.6613.186
Node.js: 20.18.1
V8: 12.8.374.38-electron.0
OS: Darwin arm64 24.1.0

@Tyriar
Copy link
Member

Tyriar commented Dec 12, 2024

@ulugbekna I can't repro, could you check devtools console for errors?

@Tyriar Tyriar added the info-needed Issue requires more information from poster label Dec 12, 2024
@ulugbekna
Copy link
Contributor Author

I don't see any related errors in dev console.

Are you also on mac and use zsh? Could you try reproing in copilot repo with all 3 tasks running and an extra terminal as in my screencast?

@Tyriar
Copy link
Member

Tyriar commented Dec 12, 2024

I was trying on Windows because the OS shouldn't matter for this. Regardless, passing to @meganrogge as main owner of tabs now

@Tyriar Tyriar assigned meganrogge and unassigned Tyriar Dec 12, 2024
@Tyriar Tyriar added terminal-tabs and removed info-needed Issue requires more information from poster labels Dec 12, 2024
@meganrogge
Copy link
Contributor

did you toggle/untoggle zen mode? if so, can repro and is a duplicate of #176085

@meganrogge meganrogge added the info-needed Issue requires more information from poster label Dec 12, 2024
@meganrogge
Copy link
Contributor

I cannot repro when I toggle panel visibility though on mac

@meganrogge
Copy link
Contributor

started seeing this recently. maybe need to bisect

Image

@meganrogge meganrogge added bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels Dec 16, 2024
@meganrogge meganrogge added this to the January 2025 milestone Dec 16, 2024
meganrogge added a commit that referenced this issue Dec 16, 2024
@vs-code-engineering vs-code-engineering bot added the unreleased Patch has not yet been released in VS Code Insiders label Dec 16, 2024
@vs-code-engineering vs-code-engineering bot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Dec 17, 2024
@meganrogge meganrogge reopened this Dec 17, 2024
@meganrogge meganrogge removed the insiders-released Patch has been released in VS Code Insiders label Dec 17, 2024
@meganrogge
Copy link
Contributor

@benibenj , git bisect points to this commit: d5746c5

Image

@benibenj
Copy link
Contributor

The only change is that toggling the panel does not give focus to the view anymore (#232790). The rendering of the view should not depend on getting focus but on visibility change

@meganrogge
Copy link
Contributor

Might want to let other view owners know about this change, I see that the debug console used to receive focus on visibility change, but now does not for example.

@meganrogge meganrogge added the regression Something that used to work is now broken label Dec 18, 2024
meganrogge added a commit that referenced this issue Dec 18, 2024
@benibenj
Copy link
Contributor

I don't mean that the views should take focus when they become visible. They layout/views logic tells your view when to be focused. The problem here seems to be that the terminal only renders correctly when it gets focused, which should not be the case. You need to make sure it renders properly when the visibility of the view changes.

I think #236502 should be reverted. It behaved correctly before

@meganrogge
Copy link
Contributor

meganrogge commented Dec 18, 2024

I think #236502 should be reverted. It behaved correctly before

It did not behave correctly before - it did not move focus to the debug console input box like it should and like it does in VS Code stable. This is an accessibility issue.

Stable:

stable.mov

Insiders:

insiders.mov

@meganrogge
Copy link
Contributor

The problem here seems to be that the terminal only renders correctly when it gets focused, which should not be the case. You need to make sure it renders properly when the visibility of the view changes.

This is not just about rendering. The terminal has historically received focus when the panel becomes visible.

@Tyriar
Copy link
Member

Tyriar commented Dec 18, 2024

This change really should have had a larger audience before it happened with pings to owners of panel views. Ctrl/cmd+j has been around forever and people depend upon it. Consistency is good, but we must have a roll out plan for something like this that changes such old/core behavior.

The problem here seems to be that the terminal only renders correctly when it gets focused, which should not be the case.

I agree with this, but the main issue @meganrogge is talking about wrt accessibility is the focus part. The layout problem is just another unfortunate affect of this change.

They layout/views logic tells your view when to be focused.

The terminal cannot do this or we would break other things like the Terminal.show(preserveFocus) extension API. At the very least
it's not as simple as just just focusing on visibility, how to get the old behavior back is not obvious to me.


imo we should revert all the changes include the original change and have the discussion about it first. This is also the lowest risk approach since most of us are about to be offline.

@Tyriar
Copy link
Member

Tyriar commented Dec 18, 2024

@benibenj FWIW the purest in me agrees with you, but I think we've had this behavior for too long and we'd be better off going in the other direction where ctrl/cmd+j shows AND focuses any panel or hides it, similar to the ctrl+` behavior. A bonus of going this route is we could use a conditional keybinding to call a showAndFocus command, so if the user really wanted to just toggle the panel they can unbound the showAndFocus one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug regression Something that used to work is now broken terminal-tabs
Projects
None yet
4 participants