Fix Popups not firing "close" event when closed programmatically

The native side sends a close event if the popup is closed either by
user input, or when disposed from JS, which is all that the "close"
method does. However, disposed native objects can not emit events, so
this is ignored and the event is missing.

Refactor Popup (migrated to TypeScript) to handle close event for all
subclasses and fire a close event explicitly if the close method is
called (before the internal dispose call).

Fix #2151

Change-Id: I403ea2c6e835560446576cd7fd5a0555640a0c8d

Author: tbuschto

src/tabris/ActionSheet.js

@@ -23,7 +23,6 @@ export default class ActionSheet extends Popup {
       _index: {enumerable: false, writable: true, value: null},
       _action: {enumerable: false, writable: true, value: null}
     });
-    this._autoDispose = true;
     this._nativeListen('select', true);
   }
 
@@ -36,14 +35,16 @@ export default class ActionSheet extends Popup {
       this._index = event.index;
       this._action = this.actions[this._index];
       super._trigger('select', Object.assign(event, {action: this._action}));
-    } else if (name === 'close') {
-      super._trigger('close', Object.assign(event, {index: this._index, action: this._action}));
-      this.dispose();
     } else {
       return super._trigger(name, event);
     }
   }
 
+  _handleClose(event) {
+    Object.assign(event, {index: this._index, action: this._action});
+    return super._handleClose(event);
+  }
+
   /** @this {import("../JsxProcessor").default} */
   [JSX.jsxFactory](Type, attributes) {
     const children = this.getChildren(attributes) || [];

src/tabris/AlertDialog.js

@@ -23,7 +23,6 @@ export default class AlertDialog extends Popup {
   constructor(properties) {
     super(properties);
     this._nativeListen('close', true);
-    this._autoDispose = true;
   }
 
   get _nativeType() {
@@ -56,21 +55,16 @@ export default class AlertDialog extends Popup {
     return super.open();
   }
 
-  _trigger(name, event) {
-    if (name === 'close') {
-      event.button = event.button || null;
-      event.texts = [];
-      if (this._contentView) {
-        event.texts = this._contentView.children().toArray().map(textInput => textInput.text);
-      }
-      if (event.button) {
-        super._trigger('close' + capitalizeFirstChar(event.button), event);
-      }
-      super._trigger('close', event);
-      this.dispose();
-    } else {
-      return super._trigger(name, event);
+  _handleClose(event) {
+    event.button = event.button || null;
+    event.texts = [];
+    if (this._contentView) {
+      event.texts = this._contentView.children().toArray().map(textInput => textInput.text);
+    }
+    if (event.button) {
+      this._trigger('close' + capitalizeFirstChar(event.button), event);
     }
+    return super._handleClose(event);
   }
 
   _dispose() {

src/tabris/DateDialog.js

@@ -23,30 +23,22 @@ export default class DateDialog extends Popup {
     super(properties);
     this._nativeListen('close', true);
     this._nativeListen('select', true);
-    this._autoDispose = true;
   }
 
   get _nativeType() {
     return 'tabris.DateDialog';
   }
 
   _trigger(name, event) {
-    if (name === 'close') {
-      this._handleCloseEvent(event);
-    } else if (name === 'select') {
+    if (name === 'select') {
       event.date = new Date(event.date);
       super._trigger('select', event);
-      this._handleCloseEvent(event);
+      return this._handleClose(event);
     } else {
       return super._trigger(name, event);
     }
   }
 
-  _handleCloseEvent(event) {
-    super._trigger('close', event);
-    this.dispose();
-  }
-
 }
 
 NativeObject.defineProperties(DateDialog.prototype, {

src/tabris/Popover.js

@@ -23,23 +23,13 @@ export default class Popover extends Popup {
     super(properties);
     Object.defineProperty(this, 'contentView', {value: createContentView()});
     this._nativeListen('close', true);
-    this._autoDispose = true;
     this._nativeSet('contentView', this.contentView.cid);
   }
 
   get _nativeType() {
     return 'tabris.Popover';
   }
 
-  _trigger(name, event) {
-    if (name === 'close') {
-      super._trigger('close', event);
-      this.dispose();
-    } else {
-      return super._trigger(name, event);
-    }
-  }
-
   _dispose() {
     if (!this.isDisposed()) {
       Composite.prototype._dispose.call(this.contentView);

src/tabris/Popup.js

@@ -0,0 +1,40 @@
+import NativeObject from './NativeObject';
+import JsxProcessor, {JSX} from './JsxProcessor';
+
+export default class Popup extends NativeObject {
+
+  open() {
+    if (this.isDisposed()) {
+      throw new Error('Can not open a popup that was disposed');
+    }
+    this._nativeCall('open', {});
+    return this;
+  }
+
+  close() {
+    this._handleClose({});
+    return this;
+  }
+
+  protected _trigger(name: string, event: {}): boolean {
+    if (name === 'close') {
+      return this._handleClose(event);
+    } else {
+      return super._trigger(name, event);
+    }
+  }
+
+  protected _handleClose(eventData: object) {
+    if (this._isDisposed || this._inDispose) {
+      return false;
+    }
+    super._trigger('close', eventData);
+    this.dispose();
+    return false;
+  }
+
+  protected [JSX.jsxFactory](this: JsxProcessor, Type: Constructor<Popup>, attributes: object) {
+    return this.createNativeObject(Type, attributes);
+  }
+
+}

src/tabris/Popup.ts

@@ -23,30 +23,22 @@ export default class TimeDialog extends Popup {
     super(properties);
     this._nativeListen('close', true);
     this._nativeListen('select', true);
-    this._autoDispose = true;
   }
 
   get _nativeType() {
     return 'tabris.TimeDialog';
   }
 
   _trigger(name, event) {
-    if (name === 'close') {
-      this._handleCloseEvent(event);
-    } else if (name === 'select') {
+    if (name === 'select') {
       event.date = new Date(event.date);
       super._trigger('select', event);
-      this._handleCloseEvent(event);
+      return this._handleClose(event);
     } else {
       return super._trigger(name, event);
     }
   }
 
-  _handleCloseEvent(event) {
-    super._trigger('close', event);
-    this.dispose();
-  }
-
 }
 
 NativeObject.defineProperties(TimeDialog.prototype, {

src/tabris/TimeDialog.js

@@ -134,6 +134,16 @@ describe('ActionSheet', () => {
       expect(actionSheet.isDisposed()).to.equal(true);
     });
 
+    it('fires close event with no selection', () => {
+      const close = spy();
+      actionSheet.onClose(close);
+
+      actionSheet.close();
+
+      expect(close).to.have.been.calledOnce;
+      expect(close).to.have.been.calledWithMatch({target: actionSheet, index: null, action: null});
+    });
+
   });
 
   describe('close event', () => {
@@ -150,14 +160,11 @@ describe('ActionSheet', () => {
     });
 
     it('fires close event with no selection', () => {
-      const closeOk = spy();
       const close = spy();
-      actionSheet.on('closeOk', closeOk);
       actionSheet.onClose(close);
 
       tabris._notify(actionSheet.cid, 'close', {});
 
-      expect(closeOk).not.to.have.been.called;
       expect(close).to.have.been.calledOnce;
       expect(close).to.have.been.calledWithMatch({target: actionSheet, index: null, action: null});
     });

test/tabris/ActionSheet.test.js

@@ -223,6 +223,16 @@ describe('AlertDialog', function() {
       expect(dialog.isDisposed()).to.equal(true);
     });
 
+    it('fires close event', function() {
+      const close = spy();
+      dialog.onClose(close);
+
+      dialog.close();
+
+      expect(close).to.have.been.calledOnce;
+      expect(close).to.have.been.calledWithMatch({target: dialog, button: null, texts: []});
+    });
+
   });
 
   describe('close event', function() {

test/tabris/AlertDialog.test.js

@@ -144,6 +144,16 @@ describe('DateDialog', function() {
       expect(dialog.isDisposed()).to.equal(true);
     });
 
+    it('fires close event', function() {
+      const close = spy();
+      dialog.onClose(close);
+
+      dialog.close();
+
+      expect(close).to.have.been.calledOnce;
+      expect(close).to.have.been.calledWithMatch({target: dialog});
+    });
+
   });
 
   describe('close event', function() {