Implement modal message API#19717
Implement modal message API#19717joaomoreno merged 5 commits intomicrosoft:masterfrom joaomoreno:modal-message-api
Conversation
| function parseMessageArguments(args: any[]): { options: vscode.MessageOptions; items: any[]; } { | ||
| const [first, ...rest] = args; | ||
|
|
||
| if (first && (typeof first === 'string' || first.title)) { |
There was a problem hiding this comment.
What if first === ''? The first check would fail even though it is a string
| } | ||
|
|
||
| private showModalMessage(severity: Severity, message: string, commands: { title: string; isCloseAffordance: boolean; handle: number; }[]): Thenable<number> { | ||
| let closeAffordanceIndex = -1; |
There was a problem hiding this comment.
Just keep a boolean? I do not see closeAffordanceIndex being used for anything but that. Or is that an oversight?
|
Pushed changes. |
| } else { | ||
| return { options: first || emptyMessageOptions, items: rest }; | ||
| } | ||
| } |
There was a problem hiding this comment.
I like the code but I don't like that it is placed here. Instead I'd propose to move it into the extHostMessageService and make its showMessage-function like: showMessage(sev, message, optionOrFirstItem, ...restItem). That would encapsulate things better IMO
There was a problem hiding this comment.
Not entirely convinced. Made rest not a spread param. Pushed for extra review.
| /** | ||
| * Indicates that this message should be modal. | ||
| */ | ||
| modal?: boolean; |
There was a problem hiding this comment.
Can we already imagine having other options here? If not we should think about a naked boolean instead
There was a problem hiding this comment.
I prefer option bags simply because it is so easy to add stuff to them later without having yet another overload. Also, there is a lot more you can do in electron dialog API (for example master/detail message) and we could also revisit our message UI to support them eventually.
|
ok to 🚢 tho I don't understand the backticks? |
|
|
|
Even though this is scheduled for the February milestone it doesn't seem to be available in the April update yet. I installed the latest vscode.d.ts but still there is no message options yet. Has this been missed, replanned or am I doing something wrong? |
5 participants
Related to #18790