-
Notifications
You must be signed in to change notification settings - Fork 37k
Standardize the breadcrumb toggle option label (fix #257550) #284486
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR standardizes the breadcrumb toggle option label to display "Breadcrumbs" instead of "Toggle Breadcrumbs" in context menus, addressing issue #257550.
- Added a
shortTitleproperty to the toggle breadcrumbs action - Updated the
toggled.titleto use the standardized "Breadcrumbs" label - Enabled short title rendering in the sticky scroll context menu
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/vs/workbench/browser/parts/editor/breadcrumbsControl.ts | Added shortTitle property and updated toggled.title to use "Breadcrumbs" label for consistency |
| src/vs/editor/contrib/stickyScroll/browser/stickyScrollController.ts | Enabled renderShortTitle option in context menu to display the new short title format |
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
| super({ | ||
| id: 'breadcrumbs.toggleFromEditorTitle', | ||
| title: localize2('notebook.toggleBreadcrumb', 'Toggle Breadcrumbs'), | ||
| title: localize2('notebook.toggleBreadcrumb', 'Breadcrumbs'), |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ToggleBreadcrumbFromEditorTitle action is inconsistent with the pattern used in ToggleLineNumberFromEditorTitle (line 119-120). While the title has been changed to "Breadcrumbs", the proper pattern would be to keep the full title as "Toggle Breadcrumbs" and add a shortTitle property (like shortTitle: localize2('cmd.toggle.short', 'Breadcrumbs')). This ensures consistency with other toggle actions and makes the action's purpose clear if the implementation changes in the future (e.g., if f1 is later set to true to allow Command Palette access).
| title: localize2('notebook.toggleBreadcrumb', 'Breadcrumbs'), | |
| title: localize2('notebook.toggleBreadcrumb', 'Toggle Breadcrumbs'), | |
| shortTitle: localize2('cmd.toggle.short', 'Breadcrumbs'), |
This PR fixes #257550
while avoiding changing how the command appears in Command Palette: