feat(dialog): Integrate with MDC List; add keyboard action handling (#…

…3594)

packages/mdc-dialog/README.md

@@ -82,9 +82,20 @@ const dialog = new MDCDialog(document.querySelector('.mdc-dialog'));
 
 > See [Importing the JS component](../../docs/importing-js.md) for more information on how to import JavaScript.
 
-> *NOTE*: MDC Dialog makes no assumptions about what will be added to the `mdc-dialog__content` element.
-> Any List, Checkboxes, etc. must also be instantiated.
-> Additionally, call `layout` on any components within the content when `MDCDialog:opened` is emitted.
+MDC Dialog makes no assumptions about what will be added to the `mdc-dialog__content` element. Any List, Checkboxes,
+etc. must also be instantiated. Additionally, call `layout` on any applicable components within the content when
+`MDCDialog:opened` is emitted.
+
+For example, to instantiate an MDC List inside of a Simple or Confirmation Dialog:
+
+```js
+import {MDCList} from '@material/list';
+const list = new MDCList(document.querySelector('.mdc-dialog .mdc-list'));
+
+dialog.listen('MDCDialog:opened', () => {
+  list.layout();
+});
+```
 
 ## Variants
 
@@ -105,10 +116,10 @@ The Simple Dialog contains a list of potential actions. It does not contain butt
  --></h2>
     <section class="mdc-dialog__content" id="my-dialog-content">
       <ul class="mdc-list">
-        <li class="mdc-list-item">
+        <li class="mdc-list-item" data-mdc-dialog-action="none">
           <span class="mdc-list-item__text">None</span>
         </li>
-        <li class="mdc-list-item">
+        <li class="mdc-list-item" data-mdc-dialog-action="callisto">
           <span class="mdc-list-item__text">Callisto</span>
         </li>
         <!-- ... -->

packages/mdc-dialog/foundation.js

@@ -134,6 +134,11 @@ class MDCDialogFoundation extends MDCFoundation {
    * @param {string=} action
    */
   close(action = '') {
+    if (!this.isOpen_) {
+      // Avoid redundant close calls (and events), e.g. from keydown on elements that inherently emit click
+      return;
+    }
+
     this.isOpen_ = false;
     this.adapter_.notifyClosing(action);
     this.adapter_.releaseFocus();
@@ -229,11 +234,13 @@ class MDCDialogFoundation extends MDCFoundation {
    * @param {!Event} evt
    * @private
    */
-  handleClick(evt) {
+  handleInteraction(evt) {
+    const isClick = evt.type === 'click';
+
     // Check for scrim click first since it doesn't require querying ancestors
-    if (this.adapter_.eventTargetHasClass(evt.target, cssClasses.SCRIM) && this.scrimClickAction_ !== '') {
+    if (isClick && this.adapter_.eventTargetHasClass(evt.target, cssClasses.SCRIM) && this.scrimClickAction_ !== '') {
       this.close(this.scrimClickAction_);
-    } else {
+    } else if (isClick || evt.key === 'Space' || evt.keyCode === 32 || evt.key === 'Enter' || evt.keyCode === 13) {
       const action = this.adapter_.getActionFromEvent(evt);
       if (action) {
         this.close(action);

packages/mdc-dialog/index.js

@@ -61,7 +61,7 @@ class MDCDialog extends MDCComponent {
     this.focusTrap_;
 
     /** @private {!Function} */
-    this.handleClick_;
+    this.handleInteraction_;
 
     /** @private {!Function} */
     this.handleDocumentKeydown_;
@@ -124,7 +124,7 @@ class MDCDialog extends MDCComponent {
   initialSyncWithDOM() {
     this.focusTrap_ = util.createFocusTrapInstance(this.container_, this.focusTrapFactory_, this.initialFocusEl_);
 
-    this.handleClick_ = this.foundation_.handleClick.bind(this.foundation_);
+    this.handleInteraction_ = this.foundation_.handleInteraction.bind(this.foundation_);
     this.handleDocumentKeydown_ = this.foundation_.handleDocumentKeydown.bind(this.foundation_);
     this.layout_ = this.layout.bind(this);
 
@@ -138,13 +138,15 @@ class MDCDialog extends MDCComponent {
       document.removeEventListener('keydown', this.handleDocumentKeydown_);
     };
 
-    this.listen('click', this.handleClick_);
+    this.listen('click', this.handleInteraction_);
+    this.listen('keydown', this.handleInteraction_);
     this.listen(strings.OPENING_EVENT, this.handleOpening_);
     this.listen(strings.CLOSING_EVENT, this.handleClosing_);
   }
 
   destroy() {
-    this.unlisten('click', this.handleClick_);
+    this.unlisten('click', this.handleInteraction_);
+    this.unlisten('keydown', this.handleInteraction_);
     this.unlisten(strings.OPENING_EVENT, this.handleOpening_);
     this.unlisten(strings.CLOSING_EVENT, this.handleClosing_);
     this.handleClosing_();

test/screenshot/spec/mdc-dialog/classes/baseline-confirmation.html

@@ -64,6 +64,7 @@ <h2 class="mdc-dialog__title test-dialog__title" id="test-dialog__title">Confirm
                                type="radio"
                                id="test-dialog-baseline-confirmation-radio-1"
                                name="test-dialog-baseline-confirmation-radio-group"
+                               value="1"
                                checked>
                         <div class="mdc-radio__background">
                           <div class="mdc-radio__outer-circle"></div>
@@ -80,7 +81,8 @@ <h2 class="mdc-dialog__title test-dialog__title" id="test-dialog__title">Confirm
                         <input class="mdc-radio__native-control"
                                type="radio"
                                id="test-dialog-baseline-confirmation-radio-2"
-                               name="test-dialog-baseline-confirmation-radio-group">
+                               name="test-dialog-baseline-confirmation-radio-group"
+                               value="2">
                         <div class="mdc-radio__background">
                           <div class="mdc-radio__outer-circle"></div>
                           <div class="mdc-radio__inner-circle"></div>

test/screenshot/spec/mdc-dialog/classes/baseline-simple.html

@@ -57,13 +57,13 @@ <h2 class="mdc-dialog__title test-dialog__title" id="test-dialog__title--simple"
                   See https://stackoverflow.com/a/23717689/467582
                 -->
                 <ul class="mdc-list" aria-orientation="vertical" style="list-style-type: none;">
-                  <li class="mdc-list-item" tabindex="0">
+                  <li class="mdc-list-item" tabindex="0" data-mdc-dialog-action="wifi">
                     <i class="material-icons mdc-list-item__graphic">network_wifi</i> Wi-Fi
                   </li>
-                  <li class="mdc-list-item" tabindex="0">
+                  <li class="mdc-list-item" tabindex="0" data-mdc-dialog-action="bluetooth">
                     <i class="material-icons mdc-list-item__graphic">bluetooth</i> Bluetooth
                   </li>
-                  <li class="mdc-list-item" tabindex="0">
+                  <li class="mdc-list-item" tabindex="0" data-mdc-dialog-action="data">
                     <i class="material-icons mdc-list-item__graphic">data_usage</i> Data usage
                   </li>
                 </ul>

test/screenshot/spec/mdc-dialog/fixture.js

@@ -28,15 +28,14 @@ window.mdc.testFixture.fontsLoaded.then(() => {
     /** @type {!MDCDialog} */
     const dialog = new mdc.dialog.MDCDialog(dialogEl);
 
-    const eventNames = [
-      strings.OPENING_EVENT, strings.OPENED_EVENT,
-      strings.CLOSING_EVENT, strings.CLOSED_EVENT,
-    ];
-
-    eventNames.forEach((eventName) => {
+    [strings.OPENING_EVENT, strings.OPENED_EVENT].forEach((eventName) => {
       dialog.listen(eventName, (evt) => console.log(eventName, evt));
     });
 
+    [strings.CLOSING_EVENT, strings.CLOSED_EVENT].forEach((eventName) => {
+      dialog.listen(eventName, (evt) => console.log(eventName, evt.detail.action, evt));
+    });
+
     // const surfaceEl = dialogEl.querySelector('.mdc-dialog__surface');
     const contentEl = dialogEl.querySelector('.mdc-dialog__content');
     const shouldScrollToBottom = dialogEl.classList.contains('test-dialog--scroll-to-bottom');
@@ -59,6 +58,12 @@ window.mdc.testFixture.fontsLoaded.then(() => {
       openButtonEl.addEventListener('click', () => dialog.open());
     }
 
+    const listEl = dialogEl.querySelector('.mdc-list');
+    if (listEl) {
+      const list = new mdc.list.MDCList(listEl);
+      dialog.listen(strings.OPENED_EVENT, () => list.layout());
+    }
+
     /* Commenting out redlines for now due to unexplained flakes
 
     dialog.listen(strings.CLOSING_EVENT, () => {

test/unit/mdc-dialog/foundation.test.js

@@ -32,6 +32,14 @@ import {cssClasses, strings, numbers} from '../../../packages/mdc-dialog/constan
 import {createMockRaf} from '../helpers/raf';
 import MDCDialogFoundation from '../../../packages/mdc-dialog/foundation';
 
+const INTERACTION_EVENTS = [
+  {type: 'click', target: {}},
+  {type: 'keydown', key: 'Enter', target: {}},
+  {type: 'keydown', keyCode: 13, target: {}},
+  {type: 'keydown', key: 'Space', target: {}},
+  {type: 'keydown', keyCode: 32, target: {}},
+];
+
 suite('MDCDialogFoundation');
 
 test('exports cssClasses', () => {
@@ -107,6 +115,8 @@ test('#open adds CSS classes', () => {
 test('#close removes CSS classes', () => {
   const {foundation, mockAdapter} = setupTest();
 
+  foundation.open();
+  td.reset();
   foundation.close();
 
   td.verify(mockAdapter.removeClass(cssClasses.OPEN));
@@ -133,6 +143,8 @@ test('#close adds the closing class to start an animation, and removes it after
   const {foundation, mockAdapter} = setupTest();
   const clock = lolex.install();
 
+  foundation.open();
+  td.reset();
   foundation.close();
 
   try {
@@ -145,28 +157,6 @@ test('#close adds the closing class to start an animation, and removes it after
   }
 });
 
-test('#isOpen returns false when the dialog has never been opened', () => {
-  const {foundation} = setupTest();
-  assert.isFalse(foundation.isOpen());
-});
-
-test('#isOpen returns true when the dialog is open', () => {
-  const {foundation} = setupTest();
-
-  foundation.open();
-
-  assert.isTrue(foundation.isOpen());
-});
-
-test('#isOpen returns false when the dialog is closed after being open', () => {
-  const {foundation} = setupTest();
-
-  foundation.open();
-  foundation.close();
-
-  assert.isFalse(foundation.isOpen());
-});
-
 test('#open activates focus trapping on the dialog surface', () => {
   const {foundation, mockAdapter} = setupTest();
   const clock = lolex.install();
@@ -185,6 +175,8 @@ test('#open activates focus trapping on the dialog surface', () => {
 test('#close deactivates focus trapping on the dialog surface', () => {
   const {foundation, mockAdapter} = setupTest();
 
+  foundation.open();
+  td.reset();
   foundation.close();
 
   td.verify(mockAdapter.releaseFocus());
@@ -209,13 +201,18 @@ test('#close emits "closing" and "closed" events', () => {
   const {foundation, mockAdapter} = setupTest();
   const clock = lolex.install();
 
+  foundation.open();
+  td.reset();
   foundation.close();
 
   try {
     td.verify(mockAdapter.notifyClosing(''), {times: 1});
     clock.tick(numbers.DIALOG_ANIMATION_CLOSE_TIME_MS);
     td.verify(mockAdapter.notifyClosed(''), {times: 1});
 
+    foundation.open();
+    td.reset();
+
     const action = 'action';
     foundation.close(action);
     td.verify(mockAdapter.notifyClosing(action), {times: 1});
@@ -226,6 +223,39 @@ test('#close emits "closing" and "closed" events', () => {
   }
 });
 
+test('#close does nothing if the dialog is already closed', () => {
+  const {foundation, mockAdapter} = setupTest();
+
+  foundation.close();
+  td.verify(mockAdapter.removeClass(cssClasses.OPEN), {times: 0});
+  td.verify(mockAdapter.removeBodyClass(cssClasses.SCROLL_LOCK), {times: 0});
+  td.verify(mockAdapter.addClass(cssClasses.CLOSING), {times: 0});
+  td.verify(mockAdapter.releaseFocus(), {times: 0});
+  td.verify(mockAdapter.notifyClosing(''), {times: 0});
+});
+
+test('#isOpen returns false when the dialog has never been opened', () => {
+  const {foundation} = setupTest();
+  assert.isFalse(foundation.isOpen());
+});
+
+test('#isOpen returns true when the dialog is open', () => {
+  const {foundation} = setupTest();
+
+  foundation.open();
+
+  assert.isTrue(foundation.isOpen());
+});
+
+test('#isOpen returns false when the dialog is closed after being open', () => {
+  const {foundation} = setupTest();
+
+  foundation.open();
+  foundation.close();
+
+  assert.isFalse(foundation.isOpen());
+});
+
 test('#open recalculates layout', () => {
   const {foundation} = setupTest();
   foundation.layout = td.func('layout');
@@ -311,52 +341,69 @@ test('#layout removes scrollable class when content is not scrollable', () => {
   mockRaf.restore();
 });
 
-test(`click closes dialog when ${strings.ACTION_ATTRIBUTE} attribute is present`, () => {
+test(`interaction closes dialog when ${strings.ACTION_ATTRIBUTE} attribute is present`, () => {
   const {foundation, mockAdapter} = setupTest();
-  const evt = {target: {}};
   const action = 'action';
   foundation.close = td.func('close');
 
-  td.when(mockAdapter.getActionFromEvent(evt)).thenReturn(action);
-  foundation.open();
-  foundation.handleClick(evt);
+  INTERACTION_EVENTS.forEach((event) => {
+    td.when(mockAdapter.getActionFromEvent(event)).thenReturn(action);
+    foundation.open();
+    foundation.handleInteraction(event);
 
-  td.verify(foundation.close(action));
+    td.verify(foundation.close(action));
+    td.reset();
+  });
 });
 
-test(`click does nothing when ${strings.ACTION_ATTRIBUTE} attribute is not present`, () => {
+test('interaction does not close dialog with action for non-activation keys', () => {
   const {foundation, mockAdapter} = setupTest();
-  const evt = {target: {}};
+  const action = 'action';
+  const event = {type: 'keydown', key: 'Escape', target: {}};
   foundation.close = td.func('close');
+  td.when(mockAdapter.getActionFromEvent(event)).thenReturn(action);
 
-  td.when(mockAdapter.getActionFromEvent(evt)).thenReturn('');
   foundation.open();
-  foundation.handleClick(evt);
+  foundation.handleInteraction(event);
 
-  td.verify(foundation.close(td.matchers.isA(String)), {times: 0});
+  td.verify(foundation.close(action), {times: 0});
+});
+
+test(`interaction does nothing when ${strings.ACTION_ATTRIBUTE} attribute is not present`, () => {
+  const {foundation, mockAdapter} = setupTest();
+  foundation.close = td.func('close');
+
+  INTERACTION_EVENTS.forEach((event) => {
+    td.when(mockAdapter.getActionFromEvent(event)).thenReturn('');
+    foundation.open();
+    foundation.handleInteraction(event);
+
+    td.verify(foundation.close(td.matchers.isA(String)), {times: 0});
+    td.reset();
+  });
 });
 
 test(`click closes dialog when ${cssClasses.SCRIM} class is present`, () => {
   const {foundation, mockAdapter} = setupTest();
-  const evt = {target: {}};
+  const evt = {type: 'click', target: {}};
   foundation.close = td.func('close');
   td.when(mockAdapter.eventTargetHasClass(evt.target, cssClasses.SCRIM)).thenReturn(true);
 
   foundation.open();
-  foundation.handleClick(evt);
+  foundation.handleInteraction(evt);
 
   td.verify(foundation.close(foundation.getScrimClickAction()));
 });
 
 test(`click does nothing when ${cssClasses.SCRIM} class is present but scrim click action is empty string`, () => {
   const {foundation, mockAdapter} = setupTest();
-  const evt = {target: {}};
+  const evt = {type: 'click', target: {}};
   foundation.close = td.func('close');
   td.when(mockAdapter.eventTargetHasClass(evt.target, cssClasses.SCRIM)).thenReturn(true);
 
   foundation.setScrimClickAction('');
   foundation.open();
-  foundation.handleClick(evt);
+  foundation.handleInteraction(evt);
 
   td.verify(foundation.close(td.matchers.isA(String)), {times: 0});
 });