Skip to content

Commit

Permalink
Add Folder to Workspace button displaying as "&Add" (#240185)
Browse files Browse the repository at this point in the history
* Add Folder to Workspace button displaying as "&Add"
Fixes #240105

* Change mnemonicButtonLabel to return both with and without mnemonic

* PR feedback

* Remove save label changes for release branch

* Fix tests

* Fix silly mistake

* Fix one more test

* Fix for mac
Unit tests on main for mac caught a bug
  • Loading branch information
alexr00 authored Feb 12, 2025
1 parent 0a5a4d6 commit e54c774
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 23 deletions.
22 changes: 16 additions & 6 deletions src/vs/base/common/labels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,17 +417,27 @@ export function mnemonicMenuLabel(label: string, forceDisableMnemonics?: boolean
* - Windows: Supported via & character (replace && with & and & with && for escaping)
* - Linux: Supported via _ character (replace && with _)
* - macOS: Unsupported (replace && with empty string)
* When forceDisableMnemonics is set, returns just the label without mnemonics.
*/
export function mnemonicButtonLabel(label: string, forceDisableMnemonics?: boolean): string {
if (isMacintosh || forceDisableMnemonics) {
return label.replace(/\(&&\w\)|&&/g, '');
export function mnemonicButtonLabel(label: string, forceDisableMnemonics: true): string;
export function mnemonicButtonLabel(label: string, forceDisableMnemonics?: false): { readonly withMnemonic: string; readonly withoutMnemonic: string };
export function mnemonicButtonLabel(label: string, forceDisableMnemonics?: boolean): { readonly withMnemonic: string; readonly withoutMnemonic: string } | string {
const withoutMnemonic = label.replace(/\(&&\w\)|&&/g, '');

if (forceDisableMnemonics) {
return withoutMnemonic;
}
if (isMacintosh) {
return { withMnemonic: withoutMnemonic, withoutMnemonic };
}

let withMnemonic: string;
if (isWindows) {
return label.replace(/&&|&/g, m => m === '&' ? '&&' : '&');
withMnemonic = label.replace(/&&|&/g, m => m === '&' ? '&&' : '&');
} else {
withMnemonic = label.replace(/&&/g, '_');
}

return label.replace(/&&/g, '_');
return { withMnemonic, withoutMnemonic };
}

export function unmnemonicLabel(label: string): string {
Expand Down
16 changes: 8 additions & 8 deletions src/vs/base/test/common/labels.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,17 +145,17 @@ suite('Labels', () => {
});

test('mnemonicButtonLabel', () => {
assert.strictEqual(labels.mnemonicButtonLabel('Hello World'), 'Hello World');
assert.strictEqual(labels.mnemonicButtonLabel(''), '');
assert.strictEqual(labels.mnemonicButtonLabel('Hello World').withMnemonic, 'Hello World');
assert.strictEqual(labels.mnemonicButtonLabel('').withMnemonic, '');
if (isWindows) {
assert.strictEqual(labels.mnemonicButtonLabel('Hello & World'), 'Hello && World');
assert.strictEqual(labels.mnemonicButtonLabel('Do &&not Save & Continue'), 'Do &not Save && Continue');
assert.strictEqual(labels.mnemonicButtonLabel('Hello & World').withMnemonic, 'Hello && World');
assert.strictEqual(labels.mnemonicButtonLabel('Do &&not Save & Continue').withMnemonic, 'Do &not Save && Continue');
} else if (isMacintosh) {
assert.strictEqual(labels.mnemonicButtonLabel('Hello & World'), 'Hello & World');
assert.strictEqual(labels.mnemonicButtonLabel('Do &&not Save & Continue'), 'Do not Save & Continue');
assert.strictEqual(labels.mnemonicButtonLabel('Hello & World').withMnemonic, 'Hello & World');
assert.strictEqual(labels.mnemonicButtonLabel('Do &&not Save & Continue').withMnemonic, 'Do not Save & Continue');
} else {
assert.strictEqual(labels.mnemonicButtonLabel('Hello & World'), 'Hello & World');
assert.strictEqual(labels.mnemonicButtonLabel('Do &&not Save & Continue'), 'Do _not Save & Continue');
assert.strictEqual(labels.mnemonicButtonLabel('Hello & World').withMnemonic, 'Hello & World');
assert.strictEqual(labels.mnemonicButtonLabel('Do &&not Save & Continue').withMnemonic, 'Do _not Save & Continue');
}
});

Expand Down
4 changes: 2 additions & 2 deletions src/vs/platform/dialogs/common/dialogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ export interface IOpenDialogOptions {
/**
* A human-readable string for the open button.
*/
readonly openLabel?: string;
readonly openLabel?: { readonly withMnemonic: string; readonly withoutMnemonic: string } | string;

/**
* Allow to select files, defaults to `true`.
Expand Down Expand Up @@ -640,7 +640,7 @@ export interface IMassagedMessageBoxOptions {
export function massageMessageBoxOptions(options: MessageBoxOptions, productService: IProductService): IMassagedMessageBoxOptions {
const massagedOptions = deepClone(options);

let buttons = (massagedOptions.buttons ?? []).map(button => mnemonicButtonLabel(button));
let buttons = (massagedOptions.buttons ?? []).map(button => mnemonicButtonLabel(button).withMnemonic);
let buttonIndeces = (options.buttons || []).map((button, index) => index);

let defaultId = 0; // by default the first button is default button
Expand Down
2 changes: 1 addition & 1 deletion src/vs/platform/dialogs/electron-main/dialogMainService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class DialogMainService implements IDialogMainService {

pickWorkspace(options: INativeOpenDialogOptions, window?: electron.BrowserWindow): Promise<string[] | undefined> {
const title = localize('openWorkspaceTitle', "Open Workspace from File");
const buttonLabel = mnemonicButtonLabel(localize({ key: 'openWorkspace', comment: ['&& denotes a mnemonic'] }, "&&Open"));
const buttonLabel = mnemonicButtonLabel(localize({ key: 'openWorkspace', comment: ['&& denotes a mnemonic'] }, "&&Open")).withMnemonic;
const filters = WORKSPACE_FILTER;

return this.doPick({ ...options, pickFiles: true, title, filters, buttonLabel }, window);
Expand Down
3 changes: 1 addition & 2 deletions src/vs/workbench/contrib/files/browser/fileImportExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import { isWeb } from '../../../../base/common/platform.js';
import { getActiveWindow, isDragEvent, triggerDownload } from '../../../../base/browser/dom.js';
import { ILogService } from '../../../../platform/log/common/log.js';
import { FileAccess, Schemas } from '../../../../base/common/network.js';
import { mnemonicButtonLabel } from '../../../../base/common/labels.js';
import { listenStream } from '../../../../base/common/stream.js';
import { DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js';
import { createSingleCallFunction } from '../../../../base/common/functional.js';
Expand Down Expand Up @@ -828,7 +827,7 @@ export class FileDownload {

const destination = await this.fileDialogService.showSaveDialog({
availableFileSystems: [Schemas.file],
saveLabel: mnemonicButtonLabel(localize('downloadButton', "Download")),
saveLabel: localize('downloadButton', "Download"),
title: localize('chooseWhereToDownload', "Choose Where to Download"),
defaultUri
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ export class SimpleFileDialog extends Disposable implements ISimpleFileDialog {
this.filePickBox.sortByLabel = false;
this.filePickBox.ignoreFocusOut = true;
this.filePickBox.ok = true;
this.filePickBox.okLabel = this.options.openLabel;
this.filePickBox.okLabel = typeof this.options.openLabel === 'string' ? this.options.openLabel : this.options.openLabel?.withoutMnemonic;
if ((this.scheme !== Schemas.file) && this.options && this.options.availableFileSystems && (this.options.availableFileSystems.length > 1) && (this.options.availableFileSystems.indexOf(Schemas.file) > -1)) {
this.filePickBox.customButton = true;
this.filePickBox.customLabel = nls.localize('remoteFileDialog.local', 'Show Local');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export class FileDialogService extends AbstractFileDialogService implements IFil
const newOptions: OpenDialogOptions & { properties: string[] } & INativeHostOptions = {
title: options.title,
defaultPath: options.defaultUri?.fsPath,
buttonLabel: options.openLabel,
buttonLabel: typeof options.openLabel === 'string' ? options.openLabel : options.openLabel?.withMnemonic,
filters: options.filters,
properties: [],
targetWindowId: getActiveWindow().vscodeWindowId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { INotificationService, Severity } from '../../../../platform/notificatio
import { IFileService } from '../../../../platform/files/common/files.js';
import { IWorkbenchEnvironmentService } from '../../environment/common/environmentService.js';
import { IFileDialogService, IDialogService } from '../../../../platform/dialogs/common/dialogs.js';
import { mnemonicButtonLabel } from '../../../../base/common/labels.js';
import { ITextFileService } from '../../textfile/common/textfiles.js';
import { IHostService } from '../../host/browser/host.js';
import { Schemas } from '../../../../base/common/network.js';
Expand Down Expand Up @@ -62,7 +61,7 @@ export abstract class AbstractWorkspaceEditingService extends Disposable impleme
availableFileSystems.unshift(Schemas.vscodeRemote);
}
let workspacePath = await this.fileDialogService.showSaveDialog({
saveLabel: mnemonicButtonLabel(localize('save', "Save")),
saveLabel: localize('save', "Save"),
title: localize('saveWorkspace', "Save Workspace"),
filters: WORKSPACE_FILTER,
defaultUri: joinPath(await this.fileDialogService.defaultWorkspacePath(), this.getNewWorkspaceName()),
Expand Down

0 comments on commit e54c774

Please sign in to comment.