fix(dialog): fixing issue with open, close, cancel events not firing (#…

…1474)

* fix(dialog): adding observer back in to send events if dialog is not a video

* fix(dialog): demo console statements need ts-ignore

* test(dialog): events

* style(dialog): whitespace

* docs(dialog): events demo

---------

Co-authored-by: Benny Powers <[email protected]>
Co-authored-by: Benny Powers - עם ישראל חי! <[email protected]>

.changeset/itchy-tools-marry.md

@@ -0,0 +1,5 @@
+---
+"@rhds/elements": patch
+---
+
+`<rh-dialog>`: ensure that `cancel`, `open`, and `closed` events fire

elements/rh-dialog/demo/events.html

@@ -0,0 +1,37 @@
+<form id="dialog-events">
+  <rh-dialog id="dialog" trigger="trigger">
+    <h2 slot="header">Modal dialog with a header</h2>
+    <p>Lorem ipsum dolor sit amet, <a href="#foo">consectetur adipisicing</a> elit, sed do eiusmod tempor incididunt
+      ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut
+      aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu
+      fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit
+      anim id est laborum.</p>
+    <rh-cta>
+      <a href="#bar">Learn more</a>
+    </rh-cta>
+  </rh-dialog>
+  <rh-button id="trigger">Open</rh-button>
+  <fieldset>
+    <legend>Events Fired</legend>
+    <output name="events">No events yet</output>
+  </fieldset>
+</form>
+
+<script type="module">
+  import '@rhds/elements/rh-button/rh-button.js';
+  import '@rhds/elements/rh-cta/rh-cta.js';
+  import '@rhds/elements/rh-dialog/rh-dialog.js';
+
+  const dialog = document.getElementById('dialog');
+  const form = document.getElementById('dialog-events');
+  const events = [];
+  form.addEventListener('submit', e => e.preventDefault());
+  const onDialogEvent = event => {
+    events.push(event.type);
+    form.elements.events.value = events.join(', ');
+  };
+  dialog.addEventListener('close', onDialogEvent);
+  dialog.addEventListener('open', onDialogEvent);
+  dialog.addEventListener('cancel', onDialogEvent);
+</script>
+

elements/rh-dialog/rh-dialog.ts

@@ -10,10 +10,11 @@ import { ScreenSizeController } from '../../lib/ScreenSizeController.js';
 
 import styles from './rh-dialog.css';
 
-import '@rhds/elements/rh-surface/rh-surface.js';
 import { query } from 'lit/decorators/query.js';
 import { ifDefined } from 'lit/directives/if-defined.js';
 
+import '@rhds/elements/rh-surface/rh-surface.js';
+
 export class DialogCancelEvent extends Event {
   constructor() {
     super('cancel', { bubbles: true, cancelable: true });
@@ -40,16 +41,6 @@ async function pauseYoutube(iframe: HTMLIFrameElement) {
   await pauseVideo(iframe);
 }
 
-function openChanged(this: RhDialog, oldValue: unknown) {
-  if (this.type === 'video' && oldValue === true && this.open === false) {
-    this.querySelector('video')?.pause?.();
-    const iframe = this.querySelector('iframe');
-    if (iframe?.src.match(/youtube/)) {
-      pauseYoutube(iframe);
-    }
-  }
-}
-
 /**
  * A dialog displays important information to users without requiring them to navigate away from the page.
  * @summary Communicates information requiring user input or action
@@ -94,7 +85,7 @@ export class RhDialog extends LitElement {
    */
   @property({ reflect: true }) position?: 'top';
 
-  @observed(openChanged)
+  @observed
   @property({ type: Boolean, reflect: true }) open = false;
 
   /** Optional ID of the trigger element */
@@ -204,9 +195,19 @@ export class RhDialog extends LitElement {
   }
 
   protected async _openChanged(oldValue?: boolean, newValue?: boolean) {
-    // loosening types to prevent running these effects in unexpected circumstances
-    // eslint-disable-next-line eqeqeq
-    if (oldValue == null || newValue == null || oldValue == newValue) {
+    if (this.type === 'video') {
+      if (oldValue === true && this.open === false) {
+        this.querySelector('video')?.pause?.();
+        const iframe = this.querySelector('iframe');
+        if (iframe?.src.match(/youtube/)) {
+          pauseYoutube(iframe);
+        }
+      }
+    } else if (oldValue == null ||
+               newValue == null ||
+               // loosening types to prevent running these effects in unexpected circumstances
+               // eslint-disable-next-line eqeqeq
+               oldValue == newValue) {
       return;
     } else if (this.open) {
       // This prevents background scroll

elements/rh-dialog/test/rh-dialog.spec.ts

@@ -1,18 +1,97 @@
-import { expect, html } from '@open-wc/testing';
+import { expect, html, oneEvent } from '@open-wc/testing';
 import { createFixture } from '@patternfly/pfe-tools/test/create-fixture.js';
+import { clickElementAtOffset } from '@patternfly/pfe-tools/test/utils.js';
+import { sendKeys } from '@web/test-runner-commands';
 import { RhDialog } from '@rhds/elements/rh-dialog/rh-dialog.js';
+import { RhButton } from '@rhds/elements/rh-button/rh-button.js';
 
-const element = html`
-  <rh-dialog></rh-dialog>
-`;
+function press(key: string) {
+  return async function() {
+    await sendKeys({ press: key });
+  };
+}
 
 describe('<rh-dialog>', function() {
   it('should upgrade', async function() {
-    const el = await createFixture<RhDialog>(element);
+    const el = await createFixture<RhDialog>(html`
+      <rh-dialog></rh-dialog>
+    `);
     const klass = customElements.get('rh-dialog');
     expect(el)
       .to.be.an.instanceOf(klass)
       .and
       .to.be.an.instanceOf(RhDialog);
   });
+  describe('with a trigger', function() {
+    let element: RhDialog;
+    let trigger: RhButton;
+    const updateComplete = () => element.updateComplete;
+    beforeEach(async function() {
+      element = await createFixture(html`
+        <rh-dialog trigger="trigger">
+          <h2 slot="header">Header</h2>
+          <p>Body</p>
+          <rh-button slot="footer">Footer Action</rh-button>
+        </rh-dialog>
+        <rh-button id="trigger">Open</rh-button>
+      `);
+      trigger = document.getElementById('trigger')!;
+    });
+    describe('clicking the trigger', function() {
+      let openEventPromise: Promise<Event>;
+      let closeEventPromise: Promise<Event>;
+      let cancelEventPromise: Promise<Event>;
+      beforeEach(function() {
+        openEventPromise = oneEvent(element, 'open');
+        closeEventPromise = oneEvent(element, 'close');
+        cancelEventPromise = oneEvent(element, 'cancel');
+      });
+      beforeEach(() => trigger.click());
+      beforeEach(updateComplete);
+      it('opens the dialog', function() {
+        expect(element.open).to.be.true;
+      });
+      it('fires "open" event', async function() {
+        const openEvent = await openEventPromise;
+        expect(openEvent.type).to.equal('open');
+      });
+      describe('pressing Escape', function() {
+        beforeEach(press('Escape'));
+        beforeEach(updateComplete);
+        it('closes the dialog', function() {
+          expect(element.open).to.be.false;
+        });
+        it('fires the cancel event', async function() {
+          const cancelEvent = await cancelEventPromise;
+          expect(cancelEvent.type).to.equal('cancel');
+        });
+      });
+      describe('clicking outside the dialog', function() {
+        beforeEach(() => clickElementAtOffset(document.body, [10, 10]));
+        beforeEach(updateComplete);
+        it('closes the dialog', function() {
+          expect(element.open).to.be.false;
+        });
+        it('fires the cancel event', async function() {
+          const cancelEvent = await cancelEventPromise;
+          expect(cancelEvent.type).to.equal('cancel');
+        });
+      });
+      describe('clicking the close button', function() {
+        // ordinarily we try our best to avoid querying the shadow root in test files
+        // in this case, we feel justified in making an exception, because the "close-button"
+        // css part is already included in the element's public API.
+        // NOTE: we query specifically for the element with that part, not by shadow class or id
+        beforeEach(() => element.shadowRoot.querySelector('[part="close-button"]')?.click());
+        beforeEach(updateComplete);
+        it('closes the dialog', function() {
+          expect(element.open).to.be.false;
+        });
+        it('fires the close event', async function() {
+          const closeEvent = await closeEventPromise;
+          expect(closeEvent.type).to.equal('close');
+        });
+      });
+    });
+  });
 });