Browse Source

chore: update app module property support (#22713)

Shelley Vohr 5 years ago
parent
commit
fc661ec56b

+ 0 - 12
docs/api/app.md

@@ -665,8 +665,6 @@ to the npm modules spec. You should usually also specify a `productName`
 field, which is your application's full capitalized name, and which will be
 preferred over `name` by Electron.
 
-**[Deprecated](modernization/property-updates.md)**
-
 ### `app.setName(name)`
 
 * `name` String
@@ -675,8 +673,6 @@ Overrides the current application's name.
 
 **Note:** This function overrides the name used internally by Electron; it does not affect the name that the OS uses.
 
-**[Deprecated](modernization/property-updates.md)**
-
 ### `app.getLocale()`
 
 Returns `String` - The current application locale. Possible return values are documented [here](locales.md).
@@ -1095,14 +1091,10 @@ On macOS, it shows on the dock icon. On Linux, it only works for Unity launcher.
 **Note:** Unity launcher requires the existence of a `.desktop` file to work,
 for more information please read [Desktop Environment Integration][unity-requirement].
 
-**[Deprecated](modernization/property-updates.md)**
-
 ### `app.getBadgeCount()` _Linux_ _macOS_
 
 Returns `Integer` - The current value displayed in the counter badge.
 
-**[Deprecated](modernization/property-updates.md)**
-
 ### `app.isUnityRunning()` _Linux_
 
 Returns `Boolean` - Whether the current desktop environment is Unity launcher.
@@ -1177,8 +1169,6 @@ technologies, such as screen readers, has been detected. See
 https://www.chromium.org/developers/design-documents/accessibility for more
 details.
 
-**[Deprecated](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
@@ -1190,8 +1180,6 @@ 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](modernization/property-updates.md)**
-
 ### `app.showAboutPanel()`
 
 Show the app's about panel options. These options can be overridden with `app.setAboutPanelOptions(options)`.

+ 0 - 6
docs/api/modernization/property-updates.md

@@ -45,9 +45,3 @@ The Electron team is currently undergoing an initiative to convert separate gett
   * `isMacTemplateImage`
 * `SystemPreferences` module
   * `appLevelAppearance`
-* `webContents` module
-  * `audioMuted`
-  * `frameRate`
-  * `userAgent`
-  * `zoomFactor`
-  * `zoomLevel`

+ 23 - 5
lib/browser/api/app.ts

@@ -17,6 +17,29 @@ let dockMenu: Electron.Menu | null = null
 Object.setPrototypeOf(App.prototype, EventEmitter.prototype)
 EventEmitter.call(app as any)
 
+// Properties.
+
+const nativeASGetter = app.isAccessibilitySupportEnabled
+const nativeASSetter = app.setAccessibilitySupportEnabled
+Object.defineProperty(App.prototype, 'accessibilitySupportEnabled', {
+  get: () => nativeASGetter.call(app),
+  set: (enabled) => nativeASSetter.call(app, enabled)
+})
+
+const nativeBCGetter = app.getBadgeCount
+const nativeBCSetter = app.setBadgeCount
+Object.defineProperty(App.prototype, 'badgeCount', {
+  get: () => nativeBCGetter.call(app),
+  set: (count) => nativeBCSetter.call(app, count)
+})
+
+const nativeNGetter = app.getName
+const nativeNSetter = app.setName
+Object.defineProperty(App.prototype, 'name', {
+  get: () => nativeNGetter.call(app),
+  set: (name) => nativeNSetter.call(app, name)
+})
+
 Object.assign(app, {
   commandLine: {
     hasSwitch: (theSwitch: string) => commandLine.hasSwitch(String(theSwitch)),
@@ -112,11 +135,6 @@ for (const name of events) {
   })
 }
 
-// Property Deprecations
-deprecate.fnToProperty(App.prototype, 'accessibilitySupportEnabled', '_isAccessibilitySupportEnabled', '_setAccessibilitySupportEnabled')
-deprecate.fnToProperty(App.prototype, 'badgeCount', '_getBadgeCount', '_setBadgeCount')
-deprecate.fnToProperty(App.prototype, 'name', '_getName', '_setName')
-
 // Deprecate allowRendererProcessReuse but only if they set it to false, no need to log if
 // they are setting it to true
 deprecate.removeProperty(app, 'allowRendererProcessReuse', [false])

+ 6 - 14
shell/browser/api/electron_api_app.cc

@@ -1413,8 +1413,8 @@ void App::BuildPrototype(v8::Isolate* isolate,
                  base::BindRepeating(&Browser::GetVersion, browser))
       .SetMethod("setVersion",
                  base::BindRepeating(&Browser::SetVersion, browser))
-      .SetMethod("_getName", base::BindRepeating(&Browser::GetName, browser))
-      .SetMethod("_setName", base::BindRepeating(&Browser::SetName, browser))
+      .SetMethod("getName", base::BindRepeating(&Browser::GetName, browser))
+      .SetMethod("setName", base::BindRepeating(&Browser::SetName, browser))
       .SetMethod("isReady", base::BindRepeating(&Browser::is_ready, browser))
       .SetMethod("whenReady", base::BindRepeating(&Browser::WhenReady, browser))
       .SetMethod("addRecentDocument",
@@ -1435,20 +1435,15 @@ void App::BuildPrototype(v8::Isolate* isolate,
       .SetMethod(
           "getApplicationNameForProtocol",
           base::BindRepeating(&Browser::GetApplicationNameForProtocol, browser))
-      .SetMethod("_setBadgeCount",
+      .SetMethod("setBadgeCount",
                  base::BindRepeating(&Browser::SetBadgeCount, browser))
-      .SetMethod("_getBadgeCount",
+      .SetMethod("getBadgeCount",
                  base::BindRepeating(&Browser::GetBadgeCount, browser))
       .SetMethod("getLoginItemSettings", &App::GetLoginItemSettings)
       .SetMethod("setLoginItemSettings",
                  base::BindRepeating(&Browser::SetLoginItemSettings, browser))
       .SetMethod("isEmojiPanelSupported",
                  base::BindRepeating(&Browser::IsEmojiPanelSupported, browser))
-      .SetProperty("badgeCount",
-                   base::BindRepeating(&Browser::GetBadgeCount, browser),
-                   base::BindRepeating(&Browser::SetBadgeCount, browser))
-      .SetProperty("name", base::BindRepeating(&Browser::GetName, browser),
-                   base::BindRepeating(&Browser::SetName, browser))
 #if defined(OS_MACOSX)
       .SetMethod("hide", base::BindRepeating(&Browser::Hide, browser))
       .SetMethod("show", base::BindRepeating(&Browser::Show, browser))
@@ -1474,9 +1469,6 @@ void App::BuildPrototype(v8::Isolate* isolate,
 #if defined(OS_MACOSX) || defined(OS_WIN)
       .SetMethod("showEmojiPanel",
                  base::BindRepeating(&Browser::ShowEmojiPanel, browser))
-      .SetProperty("accessibilitySupportEnabled",
-                   &App::IsAccessibilitySupportEnabled,
-                   &App::SetAccessibilitySupportEnabled)
 #endif
 #if defined(OS_WIN)
       .SetMethod("setUserTasks",
@@ -1503,9 +1495,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)

+ 66 - 38
spec-main/api-app-spec.ts

@@ -83,35 +83,33 @@ describe('app module', () => {
     })
   })
 
-  describe('app.name', () => {
-    it('returns the name field of package.json', () => {
-      expect(app.name).to.equal('Electron Test Main')
-    })
+  describe('app name APIs', () => {
+    it('with properties', () => {
+      it('returns the name field of package.json', () => {
+        expect(app.name).to.equal('Electron Test Main')
+      })
 
-    it('overrides the name', () => {
-      expect(app.name).to.equal('Electron Test Main')
-      app.name = 'test-name'
+      it('overrides the name', () => {
+        expect(app.name).to.equal('Electron Test Main')
+        app.name = 'test-name'
 
-      expect(app.name).to.equal('test-name')
-      app.name = 'Electron Test Main'
+        expect(app.name).to.equal('test-name')
+        app.name = 'Electron Test Main'
+      })
     })
-  })
 
-  // TODO(codebytere): remove when propertyification is complete
-  describe('app.getName()', () => {
-    it('returns the name field of package.json', () => {
-      expect(app.getName()).to.equal('Electron Test Main')
-    })
-  })
+    it('with functions', () => {
+      it('returns the name field of package.json', () => {
+        expect(app.getName()).to.equal('Electron Test Main')
+      })
 
-  // TODO(codebytere): remove when propertyification is complete
-  describe('app.setName(name)', () => {
-    it('overrides the name', () => {
-      expect(app.getName()).to.equal('Electron Test Main')
-      app.setName('test-name')
+      it('overrides the name', () => {
+        expect(app.getName()).to.equal('Electron Test Main')
+        app.setName('test-name')
 
-      expect(app.getName()).to.equal('test-name')
-      app.setName('Electron Test Main')
+        expect(app.getName()).to.equal('test-name')
+        app.setName('Electron Test Main')
+      })
     })
   })
 
@@ -554,20 +552,42 @@ describe('app module', () => {
     after(() => { app.badgeCount = 0 })
 
     describe('on supported platform', () => {
-      it('sets a badge count', function () {
-        if (platformIsNotSupported) return this.skip()
+      it('with properties', () => {
+        it('sets a badge count', function () {
+          if (platformIsNotSupported) return this.skip()
+
+          app.badgeCount = expectedBadgeCount
+          expect(app.badgeCount).to.equal(expectedBadgeCount)
+        })
+      })
 
-        app.badgeCount = expectedBadgeCount
-        expect(app.badgeCount).to.equal(expectedBadgeCount)
+      it('with functions', () => {
+        it('sets a badge count', function () {
+          if (platformIsNotSupported) return this.skip()
+
+          app.setBadgeCount(expectedBadgeCount)
+          expect(app.getBadgeCount()).to.equal(expectedBadgeCount)
+        })
       })
     })
 
     describe('on unsupported platform', () => {
-      it('does not set a badge count', function () {
-        if (platformIsSupported) return this.skip()
+      it('with properties', () => {
+        it('does not set a badge count', function () {
+          if (platformIsSupported) return this.skip()
 
-        app.badgeCount = 9999
-        expect(app.badgeCount).to.equal(0)
+          app.badgeCount = 9999
+          expect(app.badgeCount).to.equal(0)
+        })
+      })
+
+      it('with functions', () => {
+        it('does not set a badge count)', function () {
+          if (platformIsSupported) return this.skip()
+
+          app.setBadgeCount(9999)
+          expect(app.getBadgeCount()).to.equal(0)
+        })
       })
     })
   })
@@ -655,15 +675,23 @@ describe('app module', () => {
     })
   })
 
-  describe('accessibilitySupportEnabled property', () => {
-    if (process.platform === 'linux') return
+  ifdescribe(process.platform !== 'linux')('accessibilitySupportEnabled property', () => {
+    it('with properties', () => {
+      it('can set accessibility support enabled', () => {
+        expect(app.accessibilitySupportEnabled).to.eql(false)
 
-    it('returns whether the Chrome has accessibility APIs enabled', () => {
-      expect(app.accessibilitySupportEnabled).to.be.a('boolean')
+        app.accessibilitySupportEnabled = true
+        expect(app.accessibilitySupportEnabled).to.eql(true)
+      })
+    })
 
-      // TODO(codebytere): remove when propertyification is complete
-      expect(app.isAccessibilitySupportEnabled).to.be.a('function')
-      expect(app.setAccessibilitySupportEnabled).to.be.a('function')
+    it('with functions', () => {
+      it('can set accessibility support enabled', () => {
+        expect(app.isAccessibilitySupportEnabled()).to.eql(false)
+
+        app.setAccessibilitySupportEnabled(true)
+        expect(app.isAccessibilitySupportEnabled()).to.eql(true)
+      })
     })
   })