Clicking rapidly does not automatically closes the modal

If a user double clicks to open the modal, the second click gets otherwise captured by the overlay and the modal gets closed even before being visible to the user.
yapplabs · Jan 4, 2024 · 65a84d5 · 65a84d5
1 parent b4e7cb3
commit 65a84d5
Show file tree

Hide file tree

Showing 12 changed files with 106 additions and 63 deletions.
diff --git a/addon/components/basic-dialog.js b/addon/components/basic-dialog.js
@@ -5,6 +5,7 @@ import Component from '@ember/component';
 import { isEmpty } from '@ember/utils';
 import layout from '../templates/components/basic-dialog';
 import { dasherize } from '@ember/string';
+import { isIOS, clickHandlerDelay } from '../utils/config-utils';
 
 @tagName('')
 @templateLayout(layout)
@@ -107,7 +108,6 @@ export default class BasicDialog extends Component {
     if (!this.clickOutsideToClose) {
       return;
     }
-    this.makeOverlayClickableOnIOS();
 
     this.handleClick = ({ target }) => {
       // if the click has already resulted in the target
@@ -135,39 +135,28 @@ export default class BasicDialog extends Component {
       }
     };
 
+    const registerDelay = clickHandlerDelay(this);
+
     const registerClick = () =>
       document.addEventListener('click', this.handleClick);
 
     // setTimeout needed or else the click handler will catch the click that spawned this modal dialog
-    setTimeout(registerClick);
+    setTimeout(registerClick, registerDelay);
 
-    if (this.isIOS) {
+    if (isIOS) {
       const registerTouch = () =>
         document.addEventListener('touchend', this.handleClick);
-      setTimeout(registerTouch);
+      setTimeout(registerTouch, registerDelay);
     }
     super.didInsertElement(...arguments);
   }
 
   willDestroyElement() {
     document.removeEventListener('click', this.handleClick);
-    if (this.isIOS) {
+    if (isIOS) {
       document.removeEventListener('touchend', this.handleClick);
     }
-    super.willDestroyElement(...arguments);
-  }
-
-  @computed
-  get isIOS() {
-    return /iPad|iPhone|iPod/.test(navigator.userAgent);
-  }
 
-  makeOverlayClickableOnIOS() {
-    if (this.isIOS) {
-      let overlayEl = document.querySelector('div[data-emd-overlay]');
-      if (overlayEl) {
-        overlayEl.style.cursor = 'pointer';
-      }
-    }
+    super.willDestroyElement(...arguments);
   }
 }
diff --git a/addon/components/overlay.js b/addon/components/overlay.js
@@ -0,0 +1,36 @@
+import Component from '@ember/component';
+import { action } from '@ember/object';
+import { tagName, layout as templateLayout } from '@ember-decorators/component';
+import { isIOS, clickHandlerDelay } from '../utils/config-utils';
+import layout from '../templates/components/overlay';
+
+@tagName('')
+@templateLayout(layout)
+export default class OverlayComponent extends Component {
+  @action
+  _onClickOverlay(event) {
+    let { onClickOverlay } = this;
+    if (onClickOverlay) {
+      onClickOverlay(event);
+    }
+  }
+
+  @action
+  didInsert(element) {
+    const registerClick = () => {
+      element.addEventListener('click', this._onClickOverlay);
+    };
+
+    // setTimeout needed or else the click handler will catch the click that spawned this modal dialog
+    setTimeout(registerClick, clickHandlerDelay(this));
+
+    if (isIOS) {
+      element.style.cursor = 'pointer';
+    }
+  }
+
+  @action
+  willDestroyNode(element) {
+    element.removeEventListener('click', this._onClickOverlay);
+  }
+}
diff --git a/addon/templates/components/basic-dialog.hbs b/addon/templates/components/basic-dialog.hbs
@@ -2,13 +2,10 @@
   {{#if this.isOverlaySibling}}
     <div class='{{this.wrapperClassNamesString}} {{this.wrapperClass}}'>
       {{#if this.hasOverlay}}
-        <div
+        <EmberModalDialog::Overlay
           class={{this.overlayClassNamesString}}
-          onclick={{action this.onClickOverlay}}
-          tabindex='-1'
-          data-emd-overlay
-        >
-        </div>
+          @onClickOverlay={{this.onClickOverlay}}
+        />
       {{/if}}
       <EmberModalDialogPositionedContainer
         @class={{this.containerClassNamesString}}
@@ -22,11 +19,9 @@
   {{else}}
     <div class='{{this.wrapperClassNamesString}} {{this.wrapperClass}}'>
       {{#if this.hasOverlay}}
-        <div
+        <EmberModalDialog::Overlay
           class={{this.overlayClassNamesString}}
-          onclick={{action (ignore-children this.onClickOverlay)}}
-          tabindex='-1'
-          data-emd-overlay
+          @onClickOverlay={{action (ignore-children this.onClickOverlay)}}
         >
           <EmberModalDialogPositionedContainer
             @class={{this.containerClassNamesString}}
@@ -36,7 +31,7 @@
           >
             {{yield}}
           </EmberModalDialogPositionedContainer>
-        </div>
+        </EmberModalDialog::Overlay>
       {{else}}
         <EmberModalDialogPositionedContainer
           @class={{this.containerClassNamesString}}

diff --git a/addon/templates/components/liquid-dialog.hbs b/addon/templates/components/liquid-dialog.hbs
@@ -11,13 +11,10 @@
   >
     <div class='{{this.wrapperClassNamesString}} {{this.wrapperClass}}'>
       {{#if this.hasOverlay}}
-        <div
+        <EmberModalDialog::Overlay
           class={{this.overlayClassNamesString}}
-          onclick={{action this.onClickOverlay}}
-          tabindex='-1'
-          data-emd-overlay
-        >
-        </div>
+          @onClickOverlay={{this.onClickOverlay}}
+        />
       {{/if}}
       <div class={{this.containerClassNamesString}} ...attributes>
         {{yield}}
@@ -36,16 +33,14 @@
     }}
   >
     {{#if this.hasOverlay}}
-      <div
+      <EmberModalDialog::Overlay
         class={{this.overlayClassNamesString}}
-        onclick={{action (ignore-children this.onClickOverlay)}}
-        tabindex='-1'
-        data-emd-overlay
+        @onClickOverlay={{action (ignore-children this.onClickOverlay)}}
       >
         <div class={{this.containerClassNamesString}} ...attributes>
           {{yield}}
         </div>
-      </div>
+      </EmberModalDialog::Overlay>
     {{else}}
       <div class={{this.containerClassNamesString}} ...attributes>
         {{yield}}

diff --git a/addon/templates/components/liquid-tether-dialog.hbs b/addon/templates/components/liquid-tether-dialog.hbs
@@ -1,12 +1,9 @@
 {{#if this.hasOverlay}}
   <LiquidWormhole @stack='modal-overlay' @class='liquid-dialog-container'>
-    <div
+    <EmberModalDialog::Overlay
       class={{this.overlayClassNamesString}}
-      onclick={{action this.onClickOverlay}}
-      tabindex='-1'
-      data-emd-overlay
-    >
-    </div>
+      @onClickOverlay={{this.onClickOverlay}}
+    />
   </LiquidWormhole>
 {{/if}}
 <LiquidTether

diff --git a/addon/templates/components/overlay.hbs b/addon/templates/components/overlay.hbs
@@ -0,0 +1,10 @@
+<div
+  class="emd-debug"
+  tabindex='-1'
+  data-emd-overlay
+  {{did-insert this.didInsert}}
+  {{will-destroy this.willDestroyNode}}
+  ...attributes
+>
+  {{yield}}
+</div>
diff --git a/addon/templates/components/tether-dialog.hbs b/addon/templates/components/tether-dialog.hbs
@@ -1,12 +1,9 @@
 {{#if this.hasOverlay}}
   <EmberWormhole @to={{this.destinationElementId}}>
-    <div
+    <EmberModalDialog::Overlay
       class={{this.overlayClassNamesString}}
-      onclick={{action this.onClickOverlay}}
-      tabindex='-1'
-      data-emd-overlay
-    >
-    </div>
+      @onClickOverlay={{this.onClickOverlay}}
+    />
   </EmberWormhole>
 {{/if}}
 <EmberTether

diff --git a/addon/utils/config-utils.js b/addon/utils/config-utils.js
@@ -1,3 +1,5 @@
+import { getOwner } from '@ember/application';
+
 export function getDestinationElementIdFromConfig(config) {
   // if (config.environment === 'test') {
   //   return 'ember-testing';
@@ -8,3 +10,13 @@ export function getDestinationElementIdFromConfig(config) {
   modalContainerId = modalContainerId || 'modal-overlays';
   return modalContainerId;
 }
+
+export const isIOS = /iPad|iPhone|iPod/.test(navigator.userAgent);
+
+export function clickHandlerDelay(component) {
+  let ENV = getOwner(component).resolveRegistration('config:environment');
+  if (ENV.environment === 'test') {
+    return 0;
+  }
+  return 300;
+}
diff --git a/app/components/ember-modal-dialog/overlay.js b/app/components/ember-modal-dialog/overlay.js
@@ -0,0 +1 @@
+export { default } from 'ember-modal-dialog/components/overlay';
diff --git a/package.json b/package.json
@@ -39,10 +39,11 @@
   "devDependencies": {
     "@ember/jquery": "^2.0.0",
     "@ember/optional-features": "^2.0.0",
+    "@ember/render-modifiers": "^2.1.0",
     "@ember/test-helpers": "^2.6.0",
     "@embroider/test-setup": "^1.0.0",
-    "@glimmer/component": "^1.0.4",
-    "@glimmer/tracking": "^1.0.4",
+    "@glimmer/component": "^1.1.2",
+    "@glimmer/tracking": "^1.1.2",
     "babel-eslint": "^10.1.0",
     "broccoli-asset-rev": "^3.0.0",
     "ember-auto-import": "^2.4.0",

diff --git a/tests/helpers/modal-asserts.js b/tests/helpers/modal-asserts.js
@@ -28,6 +28,7 @@ export default function registerAssertHelpers(assert) {
   assert.dialogOpensAndCloses = async function (options) {
     const self = this;
     await click(options.openSelector, options.context);
+    await 
     await waitUntil(function () {
       return findContains(dialogSelector, options.dialogText);
     });

diff --git a/yarn.lock b/yarn.lock
@@ -1070,6 +1070,15 @@
     ember-cli-babel "^7.26.11"
     ember-modifier-manager-polyfill "^1.2.0"
 
+"@ember/render-modifiers@^2.1.0":
+  version "2.1.0"
+  resolved "https://registry.yarnpkg.com/@ember/render-modifiers/-/render-modifiers-2.1.0.tgz#f4fff95a8b5cfbe947ec46644732d511711c5bf9"
+  integrity sha512-LruhfoDv2itpk0fA0IC76Sxjcnq/7BC6txpQo40hOko8Dn6OxwQfxkPIbZGV0Cz7df+iX+VJrcYzNIvlc3w2EQ==
+  dependencies:
+    "@embroider/macros" "^1.0.0"
+    ember-cli-babel "^7.26.11"
+    ember-modifier-manager-polyfill "^1.2.0"
+
 "@ember/test-helpers@^2.6.0":
   version "2.6.0"
   resolved "https://registry.yarnpkg.com/@ember/test-helpers/-/test-helpers-2.6.0.tgz#d687515c6ab49ba72717fc62046970ef4a72ea9c"
@@ -1165,10 +1174,10 @@
     minimatch "^3.0.4"
     strip-json-comments "^3.1.1"
 
-"@glimmer/component@^1.0.4":
-  version "1.0.4"
-  resolved "https://registry.yarnpkg.com/@glimmer/component/-/component-1.0.4.tgz#1c85a5181615a6647f6acfaaed68e28ad7e9626e"
-  integrity sha512-sS4N8wtcKfYdUJ6O3m8nbTut6NjErdz94Ap8VB1ekcg4WSD+7sI7Nmv6kt2rdPoe363nUdjUbRBzHNWhLzraBw==
+"@glimmer/component@^1.0.4", "@glimmer/component@^1.1.2":
+  version "1.1.2"
+  resolved "https://registry.yarnpkg.com/@glimmer/component/-/component-1.1.2.tgz#892ec0c9f0b6b3e41c112be502fde073cf24d17c"
+  integrity sha512-XyAsEEa4kWOPy+gIdMjJ8XlzA3qrGH55ZDv6nA16ibalCR17k74BI0CztxuRds+Rm6CtbUVgheCVlcCULuqD7A==
   dependencies:
     "@glimmer/di" "^0.1.9"
     "@glimmer/env" "^0.1.7"
@@ -1230,10 +1239,10 @@
     "@handlebars/parser" "^1.1.0"
     simple-html-tokenizer "^0.5.10"
 
-"@glimmer/tracking@^1.0.4":
-  version "1.0.4"
-  resolved "https://registry.yarnpkg.com/@glimmer/tracking/-/tracking-1.0.4.tgz#f1bc1412fe5e2236d0f8d502994a8f88af1bbb21"
-  integrity sha512-F+oT8I55ba2puSGIzInmVrv/8QA2PcK1VD+GWgFMhF6WC97D+uZX7BFg+a3s/2N4FVBq5KHE+QxZzgazM151Yw==
+"@glimmer/tracking@^1.0.4", "@glimmer/tracking@^1.1.2":
+  version "1.1.2"
+  resolved "https://registry.yarnpkg.com/@glimmer/tracking/-/tracking-1.1.2.tgz#74e71be07b0a7066518d24044d2665d0cf8281eb"
+  integrity sha512-cyV32zsHh+CnftuRX84ALZpd2rpbDrhLhJnTXn9W//QpqdRZ5rdMsxSY9fOsj0CKEc706tmEU299oNnDc0d7tA==
   dependencies:
     "@glimmer/env" "^0.1.7"
     "@glimmer/validator" "^0.44.0"