Merge pull request #401 from cal-smith/dialog

fix(dialog): add afterDialogViewInit and improve A11Y

Author: zvonimirfras

.storybook/preview-experimental.scss

@@ -1,23 +1,38 @@
 $feature-flags: (
 	components-x: true,
-	ui-shell: true,
-	css--body: false, // we're providing our own body styles
-	css--reset: false // prevent thousands of resets being included...
+	ui-shell: true
 );
 
+$css--body: false; // we're providing our own body styles
+$css--reset: false; // prevent thousands of resets being included...
+
 .experimental {
 	@import '~carbon-components/scss/globals/scss/styles';
+	@import '~carbon-components/scss/globals/scss/css--typography';
 
 	// carbon body reset and styles
-	// @include reset;
+	@include typography;
 	@include font-family;
+	// reset
 	color: $text-01;
 	background-color: $ui-02;
 	line-height: 1;
+	box-sizing: border-box;
+	margin: 0;
+	padding: 0;
+	border: 0;
+	font-size: 100%;
+	vertical-align: baseline;
 
-	@include typography;
+	& > *,
+	& > *:before,
+	& > *:after {
+		box-sizing: inherit;
+	}
 
-	// padding/margin body reset
-	padding: 0;
-	margin: 0;
+	ul {
+		list-style: none;
+		margin: 0;
+		padding: 0;
+	}
 }

.storybook/preview.scss

@@ -1,59 +1,37 @@
 $feature-flags: (
 	components-x: false,
-	ui-shell: true,
-	css--body: false, // we're providing our own body styles
-	css--reset: false // prevent thousands of resets being included...
+	ui-shell: true
 );
 
+$css--body: false; // we're providing our own body styles
+$css--reset: false; // prevent thousands of resets being included...
+
 .carbon {
 	@import '~carbon-components/scss/globals/scss/styles';
+	@import '~carbon-components/scss/globals/scss/css--typography';
 
 	// carbon body reset and styles
-	// @include reset;
+	@include typography;
 	@include font-family;
+	//reset
 	color: $text-01;
 	background-color: $ui-02;
-	line-height: 1;
-
-	@include typography;
-
-	// padding/margin body reset
-	padding: 0;
+	box-sizing: border-box;
 	margin: 0;
-
-	// for some reason this isn't included when tabs are built. So lets just manually include it here.
-	.bx--tabs__nav-item + .bx--tabs__nav-item {
-		margin-left: 3rem;
-	}
-
-	// for some reason this isn't included when progress indicator is built. So lets just manually include it here.
-	.bx--progress-step:first-child .bx--progress-line {
-		display: none;
-	}
-
-	// for some reason all this isn't included when structured list is built. So lets just manually include it here.
-	.bx--structured-list--selection .bx--structured-list-row:hover:not(.bx--structured-list-row--header-row) {
-		background-color: $hover-row;
-		cursor: pointer;
-	}
-
-	.bx--structured-list-row:focus:not(.bx--structured-list-row--header-row) {
-		@include focus-outline('border');
-	}
-
-	.bx--structured-list-svg {
-		display: inline-block;
-		fill: transparent;
-		vertical-align: middle;
-		transition: $transition--base $carbon--standard-easing;
-	}
-
-	.bx--structured-list-row:hover .bx--structured-list-svg {
-		fill: $hover-row;
+	padding: 0;
+	border: 0;
+	font-size: 100%;
+	vertical-align: baseline;
+
+	& > *,
+	& > *:before,
+	& > *:after {
+		box-sizing: inherit;
 	}
 
-	.bx--structured-list-input:checked + .bx--structured-list-row .bx--structured-list-svg,
-	.bx--structured-list-input:checked + .bx--structured-list-td .bx--structured-list-svg {
-		fill: $brand-02;
+	ul {
+		list-style: none;
+		margin: 0;
+		padding: 0;
 	}
 }

package-lock.json

@@ -98,7 +98,7 @@
     "@types/node": "8.5.2",
     "angular2-template-loader": "0.6.2",
     "babel-loader": "8.0.4",
-    "carbon-components": "9.68.16",
+    "carbon-components": "9.70.3",
     "codelyzer": "4.5.0",
     "commitizen": "2.10.1",
     "core-js": "2.5.5",

package.json

@@ -196,15 +196,24 @@ export class Dialog implements OnInit, AfterViewInit, OnDestroy {
 		};
 
 		// settimeout to let the DOM settle before attempting to place the dialog
-		setTimeout(placeDialogInContainer);
+		// and before notifying components that the DOM is ready
+		setTimeout(() => {
+			placeDialogInContainer();
+			this.afterDialogViewInit();
+		});
 	}
 
 	/**
 	 * Empty method to be overridden by consuming classes to run any additional initialization code.
-	 * @memberof Dialog
 	 */
 	onDialogInit() {}
 
+	/**
+	 * Empty method to be overridden by consuming classes to run any additional initialization code after the view is available.
+	 * NOTE: this does _not_ guarantee the dialog will be positioned, simply that it will exist in the DOM
+	 */
+	afterDialogViewInit() {}
+
 	/**
 	 * Uses the position service to position the `Dialog` in screen space
 	 */

src/dialog/dialog.component.ts

@@ -19,7 +19,7 @@ import { DialogConfig } from "./dialog-config.interface";
 /**
  * A generic directive that can be inherited from to create dialogs (for example, a tooltip or popover)
  *
- * This class contains the relevant intilization code, specific templates, options, and additional inputs
+ * This class contains the relevant initialization code, specific templates, options, and additional inputs
  * should be specified in the derived class.
  *
  * NOTE: All child classes should add `DialogService` as a provider, otherwise they will lose context that
@@ -33,6 +33,7 @@ import { DialogConfig } from "./dialog-config.interface";
 	]
 })
 export class DialogDirective implements OnInit, OnDestroy, OnChanges {
+	static dialogCounter = 0;
 	/**
 	 * Title for the dialog
 	 * @type {string}
@@ -64,7 +65,7 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	@Input() gap = 0;
 	/**
 	 * Deprecated. Defaults to true. Use appendInline to keep dialogs within page flow
-	 * Value `true` sets Dialog be appened to the body (to break out of containers)
+	 * Value `true` appends Dialog to the body (to break out of containers)
 	 */
 	@Input() set appendToBody(v: boolean) {
 		console.log("`appendToBody` has been deprecated. Dialogs now append to the body by default.");
@@ -91,6 +92,10 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 
 	@HostBinding("attr.role") role = "button";
 	@HostBinding("attr.aria-expanded") expanded = false;
+	@HostBinding("attr.aria-haspopup") hasPopup = true;
+	@HostBinding("attr.aria-owns") get ariaOwns(): string {
+		return this.expanded ? this.dialogConfig.compID : null;
+	}
 
 	/**
 	 * Creates an instance of DialogDirective.
@@ -170,6 +175,9 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 			}
 		});
 
+		DialogDirective.dialogCounter++;
+		this.dialogConfig.compID = "dialog-" + DialogDirective.dialogCounter;
+
 		// run any code a child class may need
 		this.onDialogInit();
 	}

src/dialog/dialog.directive.ts

@@ -106,21 +106,16 @@ export class OverflowMenuPane extends Dialog implements AfterViewInit {
 		}
 	}
 
-	ngAfterViewInit() {
-		// wait for the menu to exist in the DOM before setting focus
-		// TODO: work on a more elegant solution (afterDialogInit hook maybe?)
-		setTimeout(() => {
-			const focusElementList = this.listItems();
-			focusElementList.forEach(button => {
-				// Allows user to set tabindex to 0.
-				if (button.getAttribute("tabindex") === null) {
-					button.tabIndex = -1;
-				}
-			});
-			focusElementList[0].tabIndex = 0;
-			focusElementList[0].focus();
+	afterDialogViewInit() {
+		const focusElementList = this.listItems();
+		focusElementList.forEach(button => {
+			// Allows user to set tabindex to 0.
+			if (button.getAttribute("tabindex") === null) {
+				button.tabIndex = -1;
+			}
 		});
-		super.ngAfterViewInit();
+		focusElementList[0].tabIndex = 0;
+		focusElementList[0].focus();
 	}
 
 	protected listItems() {

src/dialog/overflow-menu/overflow-menu-pane.component.ts

@@ -16,9 +16,8 @@ import { Dialog } from "./../dialog.component";
 	template: `
 		<div
 			#dialog
-			[tabindex]="tabIndex"
 			[id]="dialogConfig.compID"
-			role="tooltip"
+			[attr.role]="role"
 			class="bx--tooltip bx--tooltip--shown">
 			<span class="bx--tooltip__caret" aria-hidden="true"></span>
 			<ng-template
@@ -39,14 +38,23 @@ export class Tooltip extends Dialog {
 	 * Value is set to `true` if the `Tooltip` is to display a `TemplateRef` instead of a string.
 	 */
 	public hasContentTemplate = false;
-
-	@Input() tabIndex;
+	/**
+	 * Sets the role of the tooltip. If there's no focusable content we leave it as a `tooltip`,
+	 * if there _is_ focusable content we switch to the interactive `dialog` role.
+	 */
+	public role = "tooltip";
 	/**
 	 * Check whether there is a template for the `Tooltip` content.
 	 */
 	onDialogInit() {
 		this.hasContentTemplate = this.dialogConfig.content instanceof TemplateRef;
+	}
 
-		this.tabIndex = getFocusElementList(this.dialog.nativeElement).length > 0 ? 0 : -1;
+	afterDialogViewInit() {
+		const focusableElements = getFocusElementList(this.dialog.nativeElement);
+		if (focusableElements.length > 0) {
+			this.role = "dialog";
+			focusableElements[0].focus();
+		}
 	}
 }

src/dialog/tooltip/tooltip.component.ts

@@ -39,8 +39,6 @@ import { DialogService } from "./../dialog.service";
 	]
 })
 export class TooltipDirective extends DialogDirective {
-	static tooltipCounter = 0;
-
 	/**
 	 * The string or template content to be exposed by the tooltip.
 	 */
@@ -74,8 +72,6 @@ export class TooltipDirective extends DialogDirective {
 	 * Extends the `Dialog` component's data structure with tooltip properties.
 	 */
 	onDialogInit() {
-		TooltipDirective.tooltipCounter++;
-		this.dialogConfig.compID = "tooltip-" + TooltipDirective.tooltipCounter;
 		this.dialogConfig.content = this.ibmTooltip;
 		this.dialogConfig.type = this.tooltipType;
 	}