Add required-only modal mode for editor overlays#302290
Add required-only modal mode for editor overlays#302290joshspicer wants to merge 4 commits intomainfrom
Conversation
…nd update capabilities for specific editors
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
|
There was a problem hiding this comment.
Pull request overview
Adds a new “required-only” modal overlay mode to the workbench editor opening logic, allowing only explicitly-marked editor inputs to open in the modal editor part. This is used to ensure specific editors (e.g. Chat Customizations) can still open modally while reducing modal usage by default.
Changes:
- Introduces
workbench.editor.useModal = 'required-only'and sets it as the stable default (with migration fromessential-only). - Adds
EditorInputCapabilities.RequiresModaland gatesMODAL_GROUPcreation/usage underrequired-only. - Marks the Chat Customizations management editor input as
RequiresModal.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/workbench/services/editor/common/editorGroupFinder.ts | Gates MODAL_GROUP creation under required-only based on a new editor input capability. |
| src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationManagementEditorInput.ts | Flags the Chat Customizations management editor as requiring modal. |
| src/vs/workbench/common/editor.ts | Adds EditorInputCapabilities.RequiresModal capability bit. |
| src/vs/workbench/browser/workbench.contribution.ts | Extends setting schema, updates default, and adds config migration to required-only. |
You can also share your feedback on Copilot code review. Take the survey.
| else if (preferredGroup === MODAL_GROUP) { | ||
| const modalMode = configurationService.getValue<string>('workbench.editor.useModal'); | ||
| if (modalMode === 'required-only') { | ||
| // Only allow modal for editors that explicitly require it | ||
| const editorInput = isEditorInputWithOptions(input) ? input.editor : input; | ||
| if (isEditorInput(editorInput) && editorInput.hasCapability(EditorInputCapabilities.RequiresModal)) { | ||
| group = editorGroupService.createModalEditorPart(options?.modal) | ||
| .then(part => part.activeGroup); | ||
| } | ||
| } else if (modalMode !== 'off') { | ||
| group = editorGroupService.createModalEditorPart(options?.modal) | ||
| .then(part => part.activeGroup); | ||
| } | ||
| } |
| else if (preferredGroup === MODAL_GROUP) { | ||
| const modalMode = configurationService.getValue<string>('workbench.editor.useModal'); | ||
| if (modalMode === 'required-only') { | ||
| // Only allow modal for editors that explicitly require it | ||
| const editorInput = isEditorInputWithOptions(input) ? input.editor : input; | ||
| if (isEditorInput(editorInput) && editorInput.hasCapability(EditorInputCapabilities.RequiresModal)) { | ||
| group = editorGroupService.createModalEditorPart(options?.modal) | ||
| .then(part => part.activeGroup); | ||
| } | ||
| } else if (modalMode !== 'off') { | ||
| group = editorGroupService.createModalEditorPart(options?.modal) | ||
| .then(part => part.activeGroup); | ||
| } |
|
@joshspicer I am moving this to 1.113 since it is targeting main. Please bring it in as a candidate if needed |
bpasero
left a comment
There was a problem hiding this comment.
I do not see the advantage of this over using MODAL_GROUP when opening the editor. What problem is this trying to solve?
Lets us use modal for Chat Customizations while it may not be ready for other, existing editors.
Summary
workbench.editor.useModalmode:required-onlyproduct.quality !== 'stable' ? 'some' : 'required-only'EditorInputCapabilities.RequiresModaland gateMODAL_GROUPusage inrequired-onlyRequiresModalsome/allbehavior as supersets and add config migration fromessential-onlytorequired-onlyValidation
npm run compile-check-ts-native