-
Notifications
You must be signed in to change notification settings - Fork 37.4k
Support in-context quick pick #277712
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?
Support in-context quick pick #277712
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 adds support for in-context quick pick functionality, allowing quick pick dialogs to be positioned relative to UI elements (like buttons) rather than always appearing centered. The main implementation involves refactoring layout logic into a common module and extending the quick input system with anchor positioning.
Key changes:
- Refactored layout logic from contextview into a new common/layout.ts module for reusability
- Extended quick input API to accept anchor positioning parameters
- Integrated anchor-based positioning into the SCM history view's repository picker
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/base/common/layout.ts | New file containing extracted layout logic for positioning views relative to anchors |
| src/vs/base/browser/ui/contextview/contextview.ts | Refactored to use common layout module and export getAnchorRect utility |
| src/vs/base/browser/ui/menu/menu.ts | Updated to use refactored layout API which now returns an object instead of a number |
| src/vs/platform/quickinput/common/quickInput.ts | Added anchor property to IPickOptions and IQuickInput interfaces |
| src/vs/platform/quickinput/browser/quickInputController.ts | Implemented anchor-based positioning logic in updateLayout method |
| src/vs/workbench/contrib/scm/browser/scmHistoryViewPane.ts | Modified repository picker to pass anchor element through the call chain |
| src/vs/base/test/common/layout.test.ts | Updated tests to access position property from layout function's return value |
| } | ||
|
|
||
| override onClick(event: EventLike, preserveFocus?: boolean): void { | ||
| this.actionRunner?.run(this.action, this.element); |
Copilot
AI
Nov 16, 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 onClick override ignores both the event and preserveFocus parameters. The event parameter should be passed to the action runner as the second argument (where this.element is currently passed) to provide anchor positioning context. Based on the PR's purpose, this.element appears to be the anchor element, but it should come from the event target.
| this.actionRunner?.run(this.action, this.element); | |
| const anchor = (event && typeof event === 'object' && 'target' in event && event.target) ? event.target : this.element; | |
| this.actionRunner?.run(this.action, anchor); |
|
@lszomoru and I got this to work for some status bar items, there were challenges:
We found pre-existing issues:
Known issues:
|
Fixes #238095
Brainstorming session with @bpasero and @lszomoru:
Here's how we stitch things up:
We decorate the DOM:
Then we augment the quick pick service API to allow passing an anchor name (
anchor) that would match up withID.The quick pick service should:
[quick-pick-in-context], then remember thatIDand DOM node as state.anchor: ID, then render it in context and clear that state.