Merge pull request #815 from ing-bank/fix/overlayBugs

Fix/overlay bugs

packages/overlays/src/OverlayController.js

@@ -67,8 +67,7 @@ const supportsCSSTypedObject = window.CSS && CSS.number;
  * Note that a contentWrapperNode should be provided for [l2], [l3] and [l4]
  * In case of a global overlay ([g1]), it's enough to provide just the contentNode.
  * In case of a local overlay or a responsive overlay switching from placementMode, one should
- * always configure as if it was a local overlay.
+ * always configure as if it were a local overlay.
- *
  */
 
 export class OverlayController {
@@ -358,11 +357,14 @@ export class OverlayController {
   __setupTeardownAccessibility({ phase }) {
     if (phase === 'init') {
       this.__storeOriginalAttrs(this.contentNode, ['role', 'id']);
-      this.__storeOriginalAttrs(this.invokerNode, [
+
-        'aria-expanded',
+      if (this.invokerNode) {
-        'aria-labelledby',
+        this.__storeOriginalAttrs(this.invokerNode, [
-        'aria-describedby',
+          'aria-expanded',
-      ]);
+          'aria-labelledby',
+          'aria-describedby',
+        ]);
+      }
 
       if (!this.contentNode.id) {
         this.contentNode.setAttribute('id', this._contentId);
@@ -379,7 +381,7 @@ export class OverlayController {
         if (this.invokerNode) {
           this.invokerNode.setAttribute('aria-expanded', this.isShown);
         }
-        if (!this.contentNode.role) {
+        if (!this.contentNode.getAttribute('role')) {
           this.contentNode.setAttribute('role', 'dialog');
         }
       }
@@ -806,17 +808,28 @@ export class OverlayController {
 
     if (phase === 'show') {
       let wasClickInside = false;
-      // handle on capture phase and remember till the next task that there was an inside click
+      let wasIndirectSynchronousClick = false;
+      // Handle on capture phase and remember till the next task that there was an inside click
       this.__preventCloseOutsideClick = () => {
+        if (wasClickInside) {
+          // This occurs when a synchronous new click is triggered from a previous click.
+          // For instance, when we have a label pointing to an input, the platform triggers
+          // a new click on the input. Not taking this click into account, will hide the overlay
+          // in `__onCaptureHtmlClick`
+          wasIndirectSynchronousClick = true;
+        }
         wasClickInside = true;
         setTimeout(() => {
           wasClickInside = false;
+          setTimeout(() => {
+            wasIndirectSynchronousClick = false;
+          });
         });
       };
       // handle on capture phase and schedule the hide if needed
       this.__onCaptureHtmlClick = () => {
         setTimeout(() => {
-          if (wasClickInside === false) {
+          if (wasClickInside === false && !wasIndirectSynchronousClick) {
             this.hide();
           }
         });

packages/overlays/test/OverlayController.test.js

@@ -68,10 +68,11 @@ describe('OverlayController', () => {
         }
         if (mode === 'inline') {
           contentNode = await fixture(html`
-            <div style="z-index: ${zIndexVal} ;">
+            <div>
               I should be on top
             </div>
           `);
+          contentNode.style.zIndex = zIndexVal;
         }
         return contentNode;
       }
@@ -430,6 +431,7 @@ describe('OverlayController', () => {
         // Don't hide on inside (content) click
         ctrl.contentNode.click();
         await aTimeout();
+
         expect(ctrl.isShown).to.be.true;
 
         // Important to check if it can be still shown after, because we do some hacks inside
@@ -566,6 +568,28 @@ describe('OverlayController', () => {
         await ctrl.show();
         expect(ctrl.isShown).to.equal(true);
       });
+
+      it('doesn\'t hide on "inside label" click', async () => {
+        const contentNode = await fixture(`
+          <div>
+            <label for="test">test</label>
+            <input id="test">
+            Content
+          </div>`);
+        const labelNode = contentNode.querySelector('label[for=test]');
+        const ctrl = new OverlayController({
+          ...withGlobalTestConfig(),
+          hidesOnOutsideClick: true,
+          contentNode,
+        });
+        await ctrl.show();
+
+        // Don't hide on label click
+        labelNode.click();
+        await aTimeout();
+
+        expect(ctrl.isShown).to.be.true;
+      });
     });
 
     describe('elementToFocusAfterHide', () => {
@@ -1113,6 +1137,33 @@ describe('OverlayController', () => {
       expect(ctrl.contentNode.getAttribute('role')).to.equal('dialog');
     });
 
+    it('preserves [role] on content when present', async () => {
+      const invokerNode = await fixture('<div role="button">invoker</div>');
+      const contentNode = await fixture('<div role="menu">invoker</div>');
+      const ctrl = new OverlayController({
+        ...withLocalTestConfig(),
+        handlesAccessibility: true,
+        invokerNode,
+        contentNode,
+      });
+      expect(ctrl.contentNode.getAttribute('role')).to.equal('menu');
+    });
+
+    it('allows to not provide an invokerNode', async () => {
+      let properlyInstantiated = false;
+      try {
+        new OverlayController({
+          ...withLocalTestConfig(),
+          handlesAccessibility: true,
+          invokerNode: null,
+        });
+        properlyInstantiated = true;
+      } catch (e) {
+        throw new Error(e);
+      }
+      expect(properlyInstantiated).to.be.true;
+    });
+
     it('adds attributes inert and aria-hidden="true" on all siblings of rootNode if an overlay is shown', async () => {
       const ctrl = new OverlayController({
         ...withGlobalTestConfig(),