-
Notifications
You must be signed in to change notification settings - Fork 28.1k
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
use editMode:preview
by default for screen reader users
#182324
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.
Left a small nit about configuration service tricks
@@ -134,6 +136,17 @@ export class InteractiveEditorController implements IEditorContribution { | |||
return INTERACTIVE_EDITOR_ID; | |||
} | |||
|
|||
private _getMode(): EditMode { | |||
let editMode: EditMode = this._configurationService.getValue('interactiveEditor.editMode'); | |||
const isDefault = editMode === EditMode.LivePreview; |
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.
Better use IConfigurationService.inspect
and look at IConfigurationValue#defaultValue
and value
. That avoid hardcoding the (current) default and it will account for the fact where users explicitly configure live preview to be their choice.
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.
Sample:
vscode/src/vs/workbench/browser/parts/titlebar/windowTitle.ts
Lines 263 to 267 in efb49cc
isCustomTitleFormat(): boolean { | |
const title = this.configurationService.inspect<string>(WindowSettingNames.title); | |
const titleSeparator = this.configurationService.inspect<string>(WindowSettingNames.titleSeparator); | |
return title.value !== title.defaultValue || titleSeparator.value !== titleSeparator.defaultValue; | |
} |
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.
I will create a follow up PR
use `editMode:preview` by default for screen reader users
No description provided.