From 9c3cb55ef296254564d72ff9013813f2b03d03b5 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 4 Apr 2019 19:49:04 -0700 Subject: [PATCH] refactor: make accessibilitySupportEnabled a property on app (#17362) * refactor: make accessibilitySupport a prop on app * fix docs * update spec --- atom/browser/api/atom_api_app.cc | 7 ++++-- docs/api/app.md | 14 +++++++++++ docs/api/modernization/property-updates.md | 6 +++-- lib/browser/api/app.ts | 8 +++++-- lib/common/api/deprecate.ts | 27 ++++++++-------------- spec-main/api-app-spec.ts | 10 ++++++-- spec/api-deprecations-spec.js | 18 +++++++-------- typings/internal-electron.d.ts | 2 +- 8 files changed, 56 insertions(+), 36 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 5c0b2e749143..7048d842eed2 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -1360,6 +1360,9 @@ void App::BuildPrototype(v8::Isolate* isolate, #if defined(OS_MACOSX) || defined(OS_WIN) .SetMethod("showEmojiPanel", base::Bind(&Browser::ShowEmojiPanel, browser)) + .SetProperty("accessibilitySupportEnabled", + &App::IsAccessibilitySupportEnabled, + &App::SetAccessibilitySupportEnabled) #endif #if defined(OS_WIN) .SetMethod("setUserTasks", base::Bind(&Browser::SetUserTasks, browser)) @@ -1384,9 +1387,9 @@ void App::BuildPrototype(v8::Isolate* isolate, .SetMethod("requestSingleInstanceLock", &App::RequestSingleInstanceLock) .SetMethod("releaseSingleInstanceLock", &App::ReleaseSingleInstanceLock) .SetMethod("relaunch", &App::Relaunch) - .SetMethod("isAccessibilitySupportEnabled", + .SetMethod("_isAccessibilitySupportEnabled", &App::IsAccessibilitySupportEnabled) - .SetMethod("setAccessibilitySupportEnabled", + .SetMethod("_setAccessibilitySupportEnabled", &App::SetAccessibilitySupportEnabled) .SetMethod("disableHardwareAcceleration", &App::DisableHardwareAcceleration) diff --git a/docs/api/app.md b/docs/api/app.md index b542e0c63cfd..20984afc52a9 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -1150,6 +1150,8 @@ technologies, such as screen readers, has been detected. See https://www.chromium.org/developers/design-documents/accessibility for more details. +**[Deprecated Soon](modernization/property-updates.md)** + ### `app.setAccessibilitySupportEnabled(enabled)` _macOS_ _Windows_ * `enabled` Boolean - Enable or disable [accessibility tree](https://developers.google.com/web/fundamentals/accessibility/semantics-builtin/the-accessibility-tree) rendering @@ -1161,6 +1163,8 @@ This API must be called after the `ready` event is emitted. **Note:** Rendering accessibility tree can significantly affect the performance of your app. It should not be enabled by default. +**[Deprecated Soon](modernization/property-updates.md)** + ### `app.showAboutPanel` _macOS_ _Linux_ Show the app's about panel options. These options can be overridden with `app.setAboutPanelOptions(options)`. @@ -1337,6 +1341,16 @@ Sets the `image` associated with this dock icon. A `Menu` property that return [`Menu`](menu.md) if one has been set and `null` otherwise. Users can pass a [Menu](menu.md) to set this property. +### `app.accessibilitySupportEnabled` _macOS_ _Windows_ + +A `Boolean` property that's `true` if Chrome's accessibility support is enabled, `false` otherwise. This property will be `true` if the use of assistive technologies, such as screen readers, has been detected. Setting this property to `true` manually enables Chrome's accessibility support, allowing developers to expose accessibility switch to users in application settings. + +See [Chromium's accessibility docs](https://www.chromium.org/developers/design-documents/accessibility) for more details. Disabled by default. + +This API must be called after the `ready` event is emitted. + +**Note:** Rendering accessibility tree can significantly affect the performance of your app. It should not be enabled by default. + ### `app.isPackaged` A `Boolean` property that returns `true` if the app is packaged, `false` otherwise. For many apps, this property can be used to distinguish development and production environments. diff --git a/docs/api/modernization/property-updates.md b/docs/api/modernization/property-updates.md index a5cb11fd1c74..5b75d86414d7 100644 --- a/docs/api/modernization/property-updates.md +++ b/docs/api/modernization/property-updates.md @@ -5,9 +5,7 @@ The Electron team is currently undergoing an initiative to convert separate gett ## Candidates * `app` module - * `accessibilitySupport` * `badgeCount` - * `applicationMenu` * `name` * `dock` * `badge` @@ -56,3 +54,7 @@ The Electron team is currently undergoing an initiative to convert separate gett * `audioMuted` ## Converted Properties + +* `app` module + * `accessibilitySupport` + * `applicationMenu` diff --git a/lib/browser/api/app.ts b/lib/browser/api/app.ts index cfac9541f7d0..cc02b5b03120 100644 --- a/lib/browser/api/app.ts +++ b/lib/browser/api/app.ts @@ -38,8 +38,6 @@ Object.assign(app, { } }) -app.getFileIcon = deprecate.promisify(app.getFileIcon) - // we define this here because it'd be overly complicated to // do in native land Object.defineProperty(app, 'applicationMenu', { @@ -76,6 +74,12 @@ for (const name of events) { }) } +// Function Deprecations +app.getFileIcon = deprecate.promisify(app.getFileIcon) + +// Property Deprecations +deprecate.fnToProperty(app, 'accessibilitySupportEnabled', '_isAccessibilitySupportEnabled', '_setAccessibilitySupportEnabled') + // Wrappers for native classes. const { DownloadItem } = process.electronBinding('download_item') Object.setPrototypeOf(DownloadItem.prototype, EventEmitter.prototype) diff --git a/lib/common/api/deprecate.ts b/lib/common/api/deprecate.ts index f2bec5b9d961..d51cbcc38200 100644 --- a/lib/common/api/deprecate.ts +++ b/lib/common/api/deprecate.ts @@ -55,25 +55,18 @@ const deprecate: ElectronInternal.DeprecationUtil = { }) }, - // deprecate a getter/setter function in favor of a property - fnToProperty: (propName: string, getterFn: A, setterFn: B) => { - const getterName = getterFn.name || 'function' - const setterName = setterFn.name || 'function' - - const warnGetter = warnOnce(`${getterName} function`, `${propName} property `) - const warnSetter = warnOnce(`${setterName} function`, `${propName} property `) - - const deprecatedGetter: unknown = function (this: any) { - warnGetter() - getterFn.apply(this, arguments) + // deprecate a getter/setter function pair in favor of a property + fnToProperty: (module: any, prop: string, getter: string, setter: string) => { + const withWarnOnce = (obj: any, key: any, oldName: string, newName: string) => { + const warn = warnOnce(oldName, newName) + return (...args: any) => { + warn() + return obj[key](...args) + } } - const deprecatedSetter: unknown = function (this: any) { - warnSetter() - setterFn.apply(this, arguments) - } - - return [deprecatedGetter as A, deprecatedSetter as B] + module[getter.substr(1)] = withWarnOnce(module, getter, `${getter.substr(1)} function`, `${prop} property`) + module[setter.substr(1)] = withWarnOnce(module, setter, `${setter.substr(1)} function`, `${prop} property`) }, // remove a property with no replacement diff --git a/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts index 8a4458f6334d..8e138105e9da 100644 --- a/spec-main/api-app-spec.ts +++ b/spec-main/api-app-spec.ts @@ -634,9 +634,15 @@ describe('app module', () => { }) }) - describe('isAccessibilitySupportEnabled API', () => { + describe('accessibilitySupportEnabled property', () => { + if (process.platform === 'linux') return + it('returns whether the Chrome has accessibility APIs enabled', () => { - expect(app.isAccessibilitySupportEnabled()).to.be.a('boolean') + expect(app.accessibilitySupportEnabled).to.be.a('boolean') + + //TODO(codebytere): remove when propertyification is complete + expect(app.isAccessibilitySupportEnabled).to.be.a('function') + expect(app.setAccessibilitySupportEnabled).to.be.a('function') }) }) diff --git a/spec/api-deprecations-spec.js b/spec/api-deprecations-spec.js index b63ac0da7c8d..58e1089f234c 100644 --- a/spec/api-deprecations-spec.js +++ b/spec/api-deprecations-spec.js @@ -148,18 +148,16 @@ describe('deprecations', () => { const warnings = [] deprecations.setHandler(warning => warnings.push(warning)) - function oldGetterFn () { return 'getter' } - function oldSetterFn () { return 'setter' } + const newProp = 'newProp' + const mod = { + _oldGetterFn () { return 'getter' }, + _oldSetterFn () { return 'setter' } + } - const newProp = 'myRadProp' + deprecate.fnToProperty(mod, 'newProp', '_oldGetterFn', '_oldSetterFn') - const [ - deprecatedGetter, - deprecatedSetter - ] = deprecate.fnToProperty(newProp, oldGetterFn, oldSetterFn) - - deprecatedGetter() - deprecatedSetter() + mod['oldGetterFn']() + mod['oldSetterFn']() expect(warnings).to.have.lengthOf(2) diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index e41ac13df9b2..8c15bc758a4d 100644 --- a/typings/internal-electron.d.ts +++ b/typings/internal-electron.d.ts @@ -76,7 +76,7 @@ declare namespace ElectronInternal { log(message: string): void; function(fn: Function, newName: string): Function; event(emitter: NodeJS.EventEmitter, oldName: string, newName: string): void; - fnToProperty(propName: string, getterFn: A, setterFn: B): [A, B]; + fnToProperty(module: any, prop: string, getter: string, setter: string): void; removeProperty(object: T, propertyName: K): T; renameProperty(object: T, oldName: string, newName: K): T;