fix(dialog): prevent duplicate ids for md-dialog and md-dialog-content

Fixes #6339, Closes #6717.
angular · Jan 31, 2016 · 9ace8ec · 9ace8ec
1 parent 19966a3
commit 9ace8ec
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 15 deletions.
diff --git a/src/components/dialog/demoBasicUsage/index.html b/src/components/dialog/demoBasicUsage/index.html
@@ -4,30 +4,29 @@
     send focus back to the triggering button.
   </p>
 
-  <div class="dialog-demo-content" layout="row" layout-wrap layout-margin>
-    <md-button class="md-primary md-raised" ng-click="showAlert($event)"   flex="100"  flex-gt-md="auto">
+  <div class="dialog-demo-content" layout="row" layout-wrap layout-margin layout-align="center">
+    <md-button class="md-primary md-raised" ng-click="showAlert($event)"   >
       Alert Dialog
     </md-button>
-    <md-button class="md-primary md-raised" ng-click="showConfirm($event)"  flex="100"  flex-gt-md="auto">
+    <md-button class="md-primary md-raised" ng-click="showConfirm($event)" >
       Confirm Dialog
     </md-button>
-    <md-button class="md-primary md-raised" ng-click="showPrompt($event)"  flex="100"  flex-gt-md="auto">
+    <md-button class="md-primary md-raised" ng-click="showPrompt($event)"  >
       Prompt Dialog
     </md-button>
-    <md-button class="md-primary md-raised" ng-click="showAdvanced($event)" flex="100" flex-gt-md="auto">
+    <md-button class="md-primary md-raised" ng-click="showAdvanced($event)">
       Custom Dialog
     </md-button>
-    <div hide-gt-sm layout="row" layout-align="center center" flex="100">
-        <md-checkbox ng-model="customFullscreen" aria-label="Fullscreen Custom Dialog">Custom Dialog Fullscreen</md-checkbox>
-      </div>
-    <md-button class="md-primary md-raised" ng-click="showTabDialog($event)" flex="100"  flex-gt-md="auto">
+    <md-button class="md-primary md-raised" ng-click="showTabDialog($event)" >
       Tab Dialog
     </md-button>
   </div>
   <p class="footer">Note: The <b>Confirm</b> dialog does not use <code>$mdDialog.clickOutsideToClose(true)</code>.</p>
+  <div hide-gt-sm layout="row" layout-align="center center" flex>
+      <md-checkbox ng-model="customFullscreen" aria-label="Fullscreen Custom Dialog">Force Custom Dialog Fullscreen</md-checkbox>
+    </div>
 
-  <div ng-if="status">
-    <br/>
+  <div ng-if="status" id="status">
     <b layout="row" layout-align="center center" class="md-padding">
       {{status}}
     </b>

diff --git a/src/components/dialog/demoBasicUsage/style.css b/src/components/dialog/demoBasicUsage/style.css
@@ -12,3 +12,10 @@
     margin-top:50px;
 }
 
+button {
+    width: 200px;
+}
+
+div#status {
+    color: #c60008;
+}
diff --git a/src/components/dialog/dialog.js b/src/components/dialog/dialog.js
@@ -23,9 +23,13 @@ angular
  * Inside, use an `<md-dialog-content>` element for the dialog's content, and use
  * an `<md-dialog-actions>` element for the dialog's actions.
  *
- * * ## CSS
+ * ## CSS
  * - `.md-dialog-content` - class that sets the padding on the content as the spec file
  *
+ * ## Notes
+ * - If you specify an `id` for the `<md-dialog>`, the `<md-dialog-content>` will have the same `id`
+ * prefixed with `dialogContent_`.
+ *
  * @usage
  * ### Dialog template
  * <hljs lang="html">
@@ -887,7 +891,7 @@ function MdDialogProvider($$interimElementProvider) {
 
       var role = (options.$type === 'alert') ? 'alertdialog' : 'dialog';
       var dialogContent = element.find('md-dialog-content');
-      var dialogId = element.attr('id') || ('dialog_' + $mdUtil.nextUid());
+      var dialogContentId = ('dialogContent_' + element.attr('id')) || ('dialogContent_' + $mdUtil.nextUid());
 
       element.attr({
         'role': role,
@@ -898,8 +902,8 @@ function MdDialogProvider($$interimElementProvider) {
         dialogContent = element;
       }
 
-      dialogContent.attr('id', dialogId);
-      element.attr('aria-describedby', dialogId);
+      dialogContent.attr('id', dialogContentId);
+      element.attr('aria-describedby', dialogContentId);
 
       if (options.ariaLabel) {
         $mdAria.expect(element, 'aria-label', options.ariaLabel);

diff --git a/src/components/dialog/dialog.spec.js b/src/components/dialog/dialog.spec.js
@@ -137,6 +137,54 @@ describe('$mdDialog', function() {
       expect($document.activeElement).toBe(parent[0].querySelector('md-dialog-content'));
     }));
 
+    it('should use the prefixed id from `md-dialog` for `md-dialog-content`', inject(function ($mdDialog, $rootScope, $document) {
+      jasmine.mockElementFocus(this);
+
+      var parent = angular.element('<div>');
+
+      $mdDialog.show(
+        $mdDialog.alert({
+          template: '<md-dialog id="demoid">' +
+          '<md-dialog-content>' +
+          '<p>Muppets are the best</p>' +
+          '</md-dialog-content>' +
+          '</md-dialog>',
+          parent: parent
+        })
+      );
+
+      runAnimation();
+
+      var dialog = parent.find('md-dialog');
+      var content = parent[0].querySelector('md-dialog-content');
+
+      expect(content.id).toBe('dialogContent_' + dialog[0].id);
+    }));
+
+    it('should apply a prefixed id for `md-dialog-content`', inject(function ($mdDialog, $rootScope, $document) {
+      jasmine.mockElementFocus(this);
+
+      var parent = angular.element('<div>');
+
+      $mdDialog.show(
+        $mdDialog.alert({
+          template: '<md-dialog>' +
+          '<md-dialog-content>' +
+          '<p>Muppets are the best</p>' +
+          '</md-dialog-content>' +
+          '</md-dialog>',
+          parent: parent
+        })
+      );
+
+      runAnimation();
+
+      var dialog = parent.find('md-dialog');
+      var content = parent[0].querySelector('md-dialog-content');
+
+      expect(content.id).toMatch(/dialogContent_.*/g);
+    }));
+
     it('should remove `md-dialog-container` on mousedown mouseup and remove', inject(function($mdDialog, $rootScope, $timeout) {
       jasmine.mockElementFocus(this);
       var container, parent = angular.element('<div>');