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

Prevent closing the pinned tab when using Ctrl + W #100738

Closed
kanlukasz opened this issue Jun 22, 2020 · 20 comments
Closed

Prevent closing the pinned tab when using Ctrl + W #100738

kanlukasz opened this issue Jun 22, 2020 · 20 comments
Assignees
Labels
feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes on-testplan workbench-tabs VS Code editor tab issues

Comments

@kanlukasz
Copy link

A good example is how it works in Firefox - pinned tabs cannot be closed using Ctrl + W

When I have many tabs open (but only two are pinned), I use Ctrl + W to quickly close tab by tab (using the mouse is really unfriendly because #40290)

As a result, pinned cards are also closed. This is undesirable behavior

@bpasero
Copy link
Member

bpasero commented Jun 22, 2020

@kanlukasz you can always restore a closed tab (including it's pinned state) via Ctrl+Shift+T (Reopen closed editor), did you see that?

@bpasero bpasero added the info-needed Issue requires more information from poster label Jun 22, 2020
@kanlukasz
Copy link
Author

kanlukasz commented Jun 22, 2020

@bpasero yes, I know that and sometimes I use it.
Anyway, I still think that this option could be useful

My point of view:

Pinned tabs means something different to me than a regular tab
Usually it is something like:

  • important tab
  • something we want to keep open all the time

I use it for Git Graph , for .log files etc.

The question remains whether pinning should mean locking (maybe these should be two separate issues, I don't know if there is a ticket about locking tabs)

@bpasero bpasero added feature-request Request for new features or functionality workbench-tabs VS Code editor tab issues and removed info-needed Issue requires more information from poster labels Jun 22, 2020
@bpasero bpasero removed their assignment Jun 22, 2020
@Svish
Copy link

Svish commented Jun 29, 2020

Yes, please.

It's way too easy to accidentally close a pinned tab. Especially when you ctrl + w multiple tabs, and it doesn't happen in a linear way. When I pin a tab, I'd like it to stay pinned "forever" until I intentionally unpin and close it.

And yes, you can "undo" withctrl + shift + t, but that's only useful when you just closed it and you notice it right away. Not when you quickly close 15 files, and the pinned file was file number 8.

@Shfty
Copy link

Shfty commented Jul 23, 2020

This issue should be fixed at the source instead of being brushed off with a workaround and closed by the triage bot.

Blocking close is a standard UI expectation for pinned tabs, especially those modeled after web browsers.

@bkowshik
Copy link

Ref: #100738 (comment) 14 votes are in, just 6 more to go! 🤞

@bpasero
Copy link
Member

bpasero commented Sep 7, 2020

@kanlukasz and others, this can already be achieved today by configuring a keybinding such as:

{
    "key": "cmd+w",
    "command": "workbench.action.closeActiveEditor",
    "when": "!editorSticky"
},
{
    "key": "cmd+w",
    "command": "-workbench.action.closeActiveEditor"
}

The condition "when": "!editorSticky" will ignore this keybinding when an editor is pinned. The naming is not ideal, but historically we had a editorPinned context already that was used for a different purpose.

Is there something missing giving this solution?

@bpasero bpasero added the info-needed Issue requires more information from poster label Sep 7, 2020
@bkowshik
Copy link

bkowshik commented Sep 7, 2020

Nice! Adding the configurations shared by @bpasero worked and the behavior is as I expected.

It looks good to close this issue from my perspective.

@bkowshik
Copy link

bkowshik commented Sep 7, 2020

Had been using this for an hour now and found a bug. Below are the steps to simulate the bug:

  1. Add the config provided by @bpasero
  2. Create a pinned tab.
  3. Hit Cmd + Shift + p to open the command pallete
  4. Now, hit Ctrl + W to close the pinned tab

Ideally, the pinned tab should not have closed but it did.

  • VS Code: 1.48.2 on Mac

Screenshot 2020-09-07 at 8 09 22 PM

@Svish
Copy link

Svish commented Sep 7, 2020

Yeah, um, please no. This should be a stable and default behavior of a pinned tab. Hacking it together with overriding keyboard shortcuts is not an acceptable "solution".

Can you guys please just implement this properly? It shouldn't really be that complicated? 🙏

@bpasero
Copy link
Member

bpasero commented Sep 7, 2020

@Svish we can certainly think about making it the default for the "Close Editor" keybinding that VSCode contributes. I asked about feedback because you can change this today to get this behaviour and it is certainly helpful for me to learn if the people that have a stake in this issue find this is the right path or not. As you can see from #100738 (comment) there may even be a bug with how it works and this gives me a chance to fix the issues before landing this in the builds.

Does that make sense or do you have more to add?

@bpasero
Copy link
Member

bpasero commented Sep 7, 2020

@bkowshik thanks a lot for testing this and providing feedback. One thing that I may want to change is possibly renaming editorSticky to editorPinned and change the existing editorPinned to editorNotInPreview or similar to not confuse people. Pinned tabs are only called sticky internally but not in user facing labels.

@Svish
Copy link

Svish commented Sep 7, 2020

Asking for feedback is perfectly fine, sorry if I came off as aggressive. Just tired of software where they "fix issues" by supplying brittle and inconsistent configuration hacks that users have to randomly stumble upon in issue trackers or forum threads. And then only to find out the hack doesn't even work anymore, because the software has been updated since the hack was come up with.

Just firmly believe this should be the standard and default behavior of a "pinned tab", just like it is in for example FireFox. Otherwise it really shouldn't be called a "pinned tab", but rather a "thin tab", or something like that. 😕

@kanlukasz
Copy link
Author

@bpasero, I tested it and can confirm it works.

Additional feedback from me:
While the workbench.editor.focusRecentEditorAfterClose parameter is set to true and you will end up with a pinned tab in the closing order, you cannot close another unpinned tab (you are stopped)
I have this option set to false permanently so everything is fine for me.
But look how this works in Firefox. When the pinned card is active and you clicking Crtl + W it will take you to the closest unpinned card.

Summing up - Using your solution + setting workbench.editor.focusRecentEditorAfterClose as false, is fine for me, but maybe it is worth considering improvement in the future

@bpasero
Copy link
Member

bpasero commented Sep 9, 2020

@kanlukasz yeah I noticed how it works in Firefox and like it too. To be honest, you can easily add a keybinding to "open next editor" and assign it to "cmd+w" with the when: activeEditorIsPinned condition to get this behaviour.

Bottom line, I am not 100% sure everyone wants this behaviour or not. I have a couple of changes I plan on pushing and it will include a change for this issue and then we can continue the discussion maybe when people can try it out in insiders.

@kanlukasz
Copy link
Author

@bpasero are you sure the activeEditorIsPinned condition is correct? I tried but it not working.
My example:

{
	"key": "ctrl+w",
	"command": "workbench.action.nextEditor",
	"when": "activeEditorIsPinned"
}

@bpasero
Copy link
Member

bpasero commented Sep 10, 2020

@kanlukasz sorry, I renamed the key recently, try with editorSticky

@bpasero bpasero self-assigned this Sep 10, 2020
@bpasero bpasero removed the info-needed Issue requires more information from poster label Sep 10, 2020
@bpasero bpasero modified the milestones: Backlog, September 2020 Sep 10, 2020
@bpasero
Copy link
Member

bpasero commented Sep 10, 2020

This landed via #106385 and will be available in tomorrows insider. Pinned tabs can no longer be closed via Cmd+W. Instead, the next recently used tab will be opened (similar to FF). To make this work identical to FF you will have to configure workbench.editor.focusRecentEditorAfterClose: false, otherwise you always end up back at the pinned tab.

To overrule this change, you can simply bind Cmd+W to the new workbench.action.closeActivePinnedEditor command.

@bpasero bpasero closed this as completed Sep 10, 2020
jpda added a commit to jpda/vscode that referenced this issue Sep 14, 2020
* Add back hideHover and use on tree context menu show
Fixes microsoft#106268

* Update distro

* 💄

* explorer: Fix TrustedTypes violation

microsoft#106285

* produce deb, rpm packages

* Add loginShell (microsoft/vscode-remote-release#3593)

* chore - tweak onDidAddNotebookDocument, onDidRemoveNotebookDocument event, use ResourceMap and fix confusion between models and editors

* notebook update

* pinned tabs - update setting enum name

* Use innerText over innerHTML, microsoft#106285

* rename to IHostColorSchemeService

* API proposal for tree item icon color
Part of microsoft#103120

* chore - when target might be undefined use `target?.dispose()` over `dispose(target)`

* deprecate onDidChangeCells

* Reenable notebook smoke test microsoft#105330

* deprecate onDidChangeContent

* Add numeric values support for terminal.integrated.fontWeight

* unified onDidChangeContent

* node-pty@0.10.0-beta17

Fixes microsoft#105957

* 💄

* debt - remove _unInitializedDocuments

* remove `NotebookDocument#displayOrder` , fixes microsoft#106305

* no uninitialized documents.

* chore - update references viewlet

* debug: make serverReadyAction play nicely with js-debug

Fixes microsoft#86035
Fixes microsoft/vscode-js-debug#440

* fix rpm

* high contrast switching in browser

* Fix occasional bad custom selectbox layout
Fix microsoft#106302

* review comments

* Bump vscode-ripgrep for ARM
microsoft#6442

* Revert more specific class names for editor tokens

Reverts microsoft#103485
Fixes microsoft#106261

I believe that microsoft#103485 broke cases where the markdown renderer creates tokenized strings that are used outside of an editor's dom node (such as in hovers or in webviews)

The safest fix for now is to revert it. We can revist this and make the markdown renderer handle the tokenized output better next iteration

* remove emit selections.

* merge conflict resolve.

* fix integration tests.

* Disable errors in non-semantic supported files

Fixes microsoft#106299
Fixes microsoft#106314

Also enables js/ts features on the right side of PRs and in search results

* proper fix for microsoft#105202 (microsoft#106293)

* Only enable 'open with' on files

Fixes microsoft#106291

* Add WebviewView.description

Allow configuring the description for webview views.

This is rendered next to the title in a less prominently in the title

* Remove manual strikethroughs for deprecated properties in vscode.d.ts

Now that TS has support for `@deprecated`, these manual strike throughs should no longer be required.

* Add show method to webview view

Fixes microsoft#106085

* Skip failing test

* fix microsoft#106283

* enable test

* fix microsoft#106283

* pinnedTabSizing.standard => pinnedTabSizing.normal

* install dot net core sdk

* update distro

* tabs - align icon and text vertically centered in tab

* update distro

* distro

* fix microsoft#106308

* Update gitignore decorations when .git/info/exclude file is edited (microsoft#106270)

* detect local `exclude` file edits

* use `uri.path` to detect exclude file edits

`uri.path` uses forward slash as a path separator indepentent of
the host system, which makes it easier to use with regex

* updated searches

* editor - rename context keys variables

* fix microsoft#105999

* pinned tabs - prevent to close pinned tabs via Cmd+W (microsoft#100738)

* Reduce usage of `.innerHTML` (microsoft#106285)

* fix uninstalling extension

* remove unused import

* add `replaceNotebookMetadata` (should become `replaceMetadata`) to NotebookEditorEdit, microsoft#105283

* add `replaceNotebookMetadata`, microsoft#105283

* use textContent instead of innerHTML (for microsoft#106285)

* chore - move appyWorkspaceEdit from extHostTextEditors to (new) extHostBulkEdits and mainThreadBulkEdits

* chore - extract extHostNotebookDocument for the NotebookDocument and NotebookCell types and friends

* chore - extract ExtHostNotebookEditor into its own file

* chore - remove ExtHostNotebookEditor#uri and use document.uri instead

* chore - 💄 member order: property, ctor, method

* publish arm deb and rpm

* trusted types

related to microsoft#106285

* use async await

* distro

* update trusted types search

* trusted types - use textContent for style elements, fyi @rebornix

* fix arch

* Fix compile after merge

* Use instantiation service to create TerminalLinks

* Consolidate colon trim logic

* Avoid slice when checking colon

* Check length again

I prefer chatAt over slice as it's more obvious what's happening

* Move comment into helper function

* Update extensions/git/src/commands.ts

* Update extensions/git/src/commands.ts

* Update extensions/git/src/commands.ts

* Save prompt is shown while saving from settings split json editor (fix microsoft#106330)

* Only allow configurePlugin against main TS Server

Fixes microsoft#106346

Looks like the TS Server doesn't support this in partial mode at the moment

* Adding more explicit typings for promises

This gets us ready for TS 4.1

* Don't use async on abstract functions

* chore - use workingCopyService.isDirty instead notebook.isDirty

* Update Codicons
- Update 'pinned'
- Add 'export'
- Compress 'merge'
microsoft/vscode-codicons@5bcb1a0

* Add explicit undefined parameters / types

These errors come from the new stricter types for Promises in TS 4.1

* debt - IMainNotebookController#removeNotebookDocument

* debt - invoke resolve notebook when opening a notebook in an editor, not when loading a notebook from source

* do not need isUntitled.

* 💂 polish nb tests.

* remove selections from nb text model.

* replace changeCellLanguage to applyEdit

* fix microsoft#105916. expand metadata section if modified.

* move dirty state to NotebookEditorModel.

* chore, simply notebook text model event emitter

* refs microsoft#106285

* Add subscribers action

* Fix terminal ts 4.1 compile issues

Part of microsoft#106358

* Fix ts 4.1 issues in terminal api tests

* Update Codicons: add 'graph-left'
microsoft/vscode-codicons@dd1edb2

* initialize notebook text model data only through ctor.

* 💄

* Mark property readonly

* Enable webview commands for webview views

Fixes microsoft#105670

Previously our webview commands assumed that webviews were always going to be in an editor. This is no longer true with webview views.

To fix this, I've added an `activeWebview` to the `IWebviewService`. This  tracks the currently focused webview.

* microsoft#106358

* debug: bump js-debug-companion

* re microsoft#105735.

* re microsoft#105735. no more udpateMetadata api.

* Fix microsoft#106303

* Use destructuring to make code more clear

* Add isWritableFileSystem api

Fixes microsoft#91697

This new API checks if a given scheme allows writing

* Revert "Fix microsoft#106303"

This reverts commit 8e5eed1.

this is causing a layer check issue

* Cache webview view title across reloads

Fixes microsoft#105867

* fix some TS 4.1 errors (microsoft#106358)

* fix some TS 4.1 errors (microsoft#106358)

* fix TS 4.1 compile errors, microsoft#106358

* pinned tabs - flip default to "shrink"

* fix ts errors

related to microsoft#106358

* pinned tabs - closing pinned tab should open next non-pinned

* pinned tabs - add a tab.lastPinnedBorder color

* Adds commands for --no-verify commit variants (microsoft#106335)

* add `{allow,confirm}NoVerifyCommit`  options

* adds commit comands with no verify

* handles no verify command variants

* handle no verify commit option

* only display no verify variants when option is set

* trusted types

related to microsoft#106395

* more TS 4.1 fixes (microsoft#106358)

* update trusted types search

* Fix TS 4.1 errors for tasks and remote explorer
Part of microsoft#106358

* Adress microsoft#106358: Fix TS 4.1 errors in codebase

* debt - simplify metadata edit because we now have CellEditType.DocumentMetadata

* Fix Trusted Types violations (round #2) microsoft#106395

* debug: return result of a msg to debug adapter can be undefined

* add ExtHostFileSystemInfo which knows what schemes are reserved and which are used, microsoft#91697

* fixes microsoft#106334

* web - fix bad credentials lookup

* Correct path to code-workspace.xml

Fixes microsoft#106384

* Multi git executable paths (microsoft#85954)

* Multi git executable path

* update `git.path` setting description

* 💄

Co-authored-by: João Moreno <joao.moreno@microsoft.com>

* Correct linux code-workspace path

* fixes microsoft#104047

* Add defaultUriScheme to path service (microsoft#106400)

Fixes microsoft/vscode-internalbacklog#1179

* 💄

* Fix  microsoft#106303

* Avoid innerHTML (microsoft#106395)

* Avoid innerHTML (microsoft#106395)

* debt - REMOTE_HOST_SCHEME => Schemas.vscodeRemote

* fixes microsoft#106355

* pathService - defaultUriScheme() => defaultUriScheme

* Adjust active terminal tab when active tab moves (microsoft#106413)

Fixes microsoft#106300

* debt - adopt defaultUriScheme also for userHome

* debt - adopt defaultUriScheme over hardcoded vscode-remote in toLocalResource

* some integration tests for notebook editing, microsoft#105283

* refs microsoft#106358

* Bump yargs-parser in /extensions/markdown-language-features (microsoft#106373)

Bumps [yargs-parser](https://github.com/yargs/yargs-parser) from 13.1.1 to 13.1.2.
- [Release notes](https://github.com/yargs/yargs-parser/releases)
- [Changelog](https://github.com/yargs/yargs-parser/blob/master/docs/CHANGELOG-full.md)
- [Commits](https://github.com/yargs/yargs-parser/commits)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump yargs-parser in /extensions/css-language-features/server

Bumps [yargs-parser](https://github.com/yargs/yargs-parser) from 13.1.1 to 13.1.2.
- [Release notes](https://github.com/yargs/yargs-parser/releases)
- [Changelog](https://github.com/yargs/yargs-parser/blob/master/docs/CHANGELOG-full.md)
- [Commits](https://github.com/yargs/yargs-parser/commits)

Signed-off-by: dependabot[bot] <support@github.com>

* Remove arrays.findIndex

For microsoft#103454

This should be a direct map to the `.findIndex` mathod

* Use textContnet for style element

For microsoft#106395

* use textcontent in menu css
refs microsoft#106395

* Fix one innerHTML usage microsoft#106395

* Use `@example` tags in vscode.d.ts (microsoft#106352)

`@example` is the standard way to document code examples. This also gets us syntax highlighting of code examples in hovers

* - reload only local user configuration after initi
- add perf mark up and logs

* re microsoft#105735. batch apply edits.

* notebook text model initialization does not increment version

* private outputs slice and unknown change.

* applyEdit supports begin/end selections.

* replace insertCell with applyEdit.

* do not copy execution related metadata

* 💄

* fix build.

* Document view.type contribution

Fixes microsoft#105821

* Improve views contribution point

- add required properties
- add default snippet
- use `markdownDescription` for markdown string

* Replace our arrays.find with stdlib find

For microsoft#103454

* Pin blob storage dep
see Azure/azure-sdk-for-js#11187

* unit tests for batched edits.

* remove spliceNotebook api from textmodel.

* update exploration branch

* Fix some trusted type violations, microsoft#106395

* fix fonts in monaco menu

* update distro

* some jsdoc for microsoft#54938

* try to fix build (linux)

* electron - set spellcheck: false again for windows

* update search file, microsoft#106395

* 🆙 web playground

* Trusted types - don't use innerHTML for rapid render, microsoft#106395

* Remove Schemas.vscodeRemote from simple file dialog

* debt - remove some any casts from window

* update distro

* fix linux build

* argh

* proxy authentication does not work on 1.49 (microsoft#106434)

* do not use hasClass and first

microsoft#103454

* debug and explroer: do not use dom.addClass, dom.toggleClass

* do not use deprecated dom helper methods

microsoft#103454

* adopt new amd loader with support for TrustedScriptURL, add typings for TrustedTypesFactory et al, microsoft#106396

* explorer: Should maintain row focus after deleting a file

fixes microsoft#71315

* Update Codicons: add 'magnet' icon
microsoft/vscode-codicons@4c61155

* Remove unused 'SettingSearch' issue type

* notebook document data loss.

* cell language should not be freezed.

* Add preferred_username to the list of msft token claims (microsoft#106511)

* debug: update js-debug

* fix microsoft#106430.

* hide outputs if it is transient.

* Add optional typing for webview state in WebviewPanelSerializer

This makes it easier for extensions to correctly type their state if they wish

* Add comment to WebviewViewResolveContext

* use optional chaining

* Use `Set` instead of array

Sets should offer faster checking to see if a property has been seen

* Create webview.web.contribution

Fixes microsoft#106516

Creates an explicit contribution file for web. This makes sure we only don't register the `IWebviewService` twice. Not 100% sure this fixes the issue since I couldn't repo the original bug with our oss builds

* Revert "API proposal for tree item icon color"

This reverts commit 52e557f.

* Skip formatting when during format-on-save, the configured formatter cannot be found (continue to show silent notification), microsoft#106376

* don't use renderCodicons any more, microsoft#105799

* remove old renderCodicons-function, rename renderCodiconsAsElement to renderCodicons

* NotebookEditorEdit-api changes, microsoft#105283

* WorkspaceEdit-api changes, microsoft#105283

* adopt notebook integration tests, microsoft#105283

* add NotebookCell#index, microsoft#106637

* fix delay issue for provideCodeLenses, microsoft#106267

* rename RunOnceScheduler#timeout to delay

* use debian stretch images (microsoft#106656)

* remove deprecated function calls

related to microsoft#103454

* workaround, maybe fix for microsoft#106657

* update search files

* debt - make class list utils functions so that @deprecated works for them

* fixes microsoft#106406

* notebook - when creating a notebook, check that no notebook with another viewtype exists

* fix bad classList usage

* add regression test for microsoft#106657

* fixes microsoft#86180

* fixes microsoft#100172

Co-authored-by: Alex Ross <alros@microsoft.com>
Co-authored-by: Daniel Imms <daimms@microsoft.com>
Co-authored-by: João Moreno <joao.moreno@microsoft.com>
Co-authored-by: isidor <inikolic@microsoft.com>
Co-authored-by: Christof Marti <chrmarti@microsoft.com>
Co-authored-by: Johannes Rieken <johannes.rieken@gmail.com>
Co-authored-by: Benjamin Pasero <benjpas@microsoft.com>
Co-authored-by: Martin Aeschlimann <martinae@microsoft.com>
Co-authored-by: rebornix <penn.lv@gmail.com>
Co-authored-by: Rob Lourens <roblourens@gmail.com>
Co-authored-by: IllusionMH <illusionmh@gmail.com>
Co-authored-by: Daniel Imms <tyriar@tyriar.com>
Co-authored-by: Connor Peet <connor@peet.io>
Co-authored-by: Matt Bierner <matb@microsoft.com>
Co-authored-by: Jean Pierre <jeanp413@hotmail.com>
Co-authored-by: Peng Lyu <penlv@microsoft.com>
Co-authored-by: Sandeep Somavarapu <sasomava@microsoft.com>
Co-authored-by: Vyacheslav Pukhanov <vyacheslav.pukhanov@gmail.com>
Co-authored-by: Alex Dima <alexdima@microsoft.com>
Co-authored-by: João Moreno <mail@joaomoreno.com>
Co-authored-by: Miguel Solorio <miguel.solorio@microsoft.com>
Co-authored-by: SteVen Batten <steven.m.batten@outlook.com>
Co-authored-by: Jackson Kearl <jakearl@microsoft.com>
Co-authored-by: Rachel Macfarlane <ramacfar@microsoft.com>
Co-authored-by: Dirk Baeumer <dirkb@microsoft.com>
Co-authored-by: WhizSid <whizsid@aol.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: deepak1556 <hop2deep@gmail.com>
Co-authored-by: Oleg Demchenko <oldemche@microsoft.com>
@bpasero bpasero mentioned this issue Sep 25, 2020
3 tasks
@bpasero bpasero added on-testplan on-release-notes Issue/pull request mentioned in release notes labels Sep 25, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes on-testplan workbench-tabs VS Code editor tab issues
Projects
None yet
Development

No branches or pull requests

6 participants
@Svish @bpasero @Shfty @bkowshik @kanlukasz and others