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

Closing inactive editors will still update breadcrumb #99041

Closed
jrieken opened this issue Jun 2, 2020 · 8 comments
Closed

Closing inactive editors will still update breadcrumb #99041

jrieken opened this issue Jun 2, 2020 · 8 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debt Code quality issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded workbench-tabs VS Code editor tab issues
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Jun 2, 2020

Testing #98019

  • see attached GIF
  • open and pin an editor, make sure it has breadcrumbs
  • press cmd+w
  • editor stays but breadcrumbs flash

Jun-02-2020 11-49-24

@bpasero
Copy link
Member

bpasero commented Jun 2, 2020

@jrieken hm, Cmd+W at least should always close an editor, even if pinned. Did you assign anything special to Cmd+W?

@bpasero bpasero added this to the May 2020 milestone Jun 2, 2020
@bpasero bpasero added the info-needed Issue requires more information from poster label Jun 2, 2020
@jrieken
Copy link
Member Author

jrieken commented Jun 2, 2020

Yeah, looks like I have "close all editors in group"

Screenshot 2020-06-02 at 13 55 11

@bpasero bpasero added the workbench-tabs VS Code editor tab issues label Jun 2, 2020
@bpasero
Copy link
Member

bpasero commented Jun 2, 2020

@jrieken that would explain it, and is covered by #99037

I can look why the breadcrumb flickers, that seems wrong to me.

@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels Jun 2, 2020
@bpasero bpasero removed their assignment Jun 3, 2020
@bpasero
Copy link
Member

bpasero commented Jun 3, 2020

It looks like we would update breadcrumbs even if no editors are closing, I fixed that with 64955f1

Looking at

this.updateBreadcrumbsControl();
we still might have cases where we call updateBreadcrumbsControl even if the editors that close are not the active one. Leaving that up to you to clean up, e.g. maybe this should only call when the active editor is part of the ones to close.

@jrieken jrieken modified the milestones: May 2020, June 2020 Jun 3, 2020
@jrieken jrieken added the debt Code quality issues label Jun 3, 2020
@bpasero bpasero changed the title "Closing" a pinned editor flashes breadcrumbs Closing inactive editors will still update breadcrumb Jun 4, 2020
@jrieken
Copy link
Member Author

jrieken commented Jun 8, 2020

I would need to know if the active tab was closed or not but it seems that this.group.activeEditor is already set to the new, active editor when reaching this code:

closeEditor(editor: IEditorInput): void {

@bpasero Am I missing something or this this too far down the line?

@jrieken
Copy link
Member Author

jrieken commented Jun 8, 2020

nvm - looks like openEditor is called always before close (when closing a single tab) and then breadcrumbs are already updated. Only when closing all tab special breadcrumb updating is needed...

@jrieken jrieken closed this as completed in cf73ba7 Jun 8, 2020
@bpasero
Copy link
Member

bpasero commented Jun 8, 2020

@jrieken closeEditors may also be called without the active one being included, e.g. "Close Others". How about this code:

closeEditors(editors: IEditorInput[]): void {
	this.handleClosedEditors();

	if (this.group.activeEditor && editors.indexOf(this.group.activeEditor) !== -1) {
		this.updateBreadcrumbsControl();
	}
}

To make sure breadcrumbs update when the list of closed editors includes the active one. I can see that reduces flicker when running "Close Others".

@jrieken
Copy link
Member Author

jrieken commented Jun 9, 2020

For closeEditor I see the same. Once the call reached me, the active editor is already updated and hence never in the list closed editors. Meaning, I'd never update the breadcrumbs here... Tho, it might be enough to only update the breadcrumbs when no more editor is around (which means hide breadcrumbs)

@bpasero bpasero added the verified Verification succeeded label Jul 2, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug debt Code quality issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded workbench-tabs VS Code editor tab issues
Projects
None yet
Development

No branches or pull requests

3 participants
@bpasero @jrieken and others