Browse Source

refactor: make accessibilitySupportEnabled a property on app (#17362)

* refactor: make accessibilitySupport a prop on app

* fix docs

* update spec
Shelley Vohr 6 years ago
parent
commit
9c3cb55ef2

+ 5 - 2
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)

+ 14 - 0
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.

+ 4 - 2
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`

+ 6 - 2
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)

+ 10 - 17
lib/common/api/deprecate.ts

@@ -55,25 +55,18 @@ const deprecate: ElectronInternal.DeprecationUtil = {
     })
   },
 
-  // deprecate a getter/setter function in favor of a property
-  fnToProperty: <A extends Function, B extends Function>(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)
-    }
-
-    const deprecatedSetter: unknown = function (this: any) {
-      warnSetter()
-      setterFn.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)
+      }
     }
 
-    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

+ 8 - 2
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')
     })
   })
 

+ 8 - 10
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 = 'myRadProp'
+    const newProp = 'newProp'
+    const mod = {
+      _oldGetterFn () { return 'getter' },
+      _oldSetterFn () { return 'setter' }
+    }
 
-    const [
-      deprecatedGetter,
-      deprecatedSetter
-    ] = deprecate.fnToProperty(newProp, oldGetterFn, oldSetterFn)
+    deprecate.fnToProperty(mod, 'newProp', '_oldGetterFn', '_oldSetterFn')
 
-    deprecatedGetter()
-    deprecatedSetter()
+    mod['oldGetterFn']()
+    mod['oldSetterFn']()
 
     expect(warnings).to.have.lengthOf(2)
 

+ 1 - 1
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<A extends Function, B extends Function>(propName: string, getterFn: A, setterFn: B): [A, B];
+    fnToProperty(module: any, prop: string, getter: string, setter: string): void;
     removeProperty<T, K extends (keyof T & string)>(object: T, propertyName: K): T;
     renameProperty<T, K extends (keyof T & string)>(object: T, oldName: string, newName: K): T;