Merge pull request #223 from cal-smith/appendto

fix: change everything to append to the body by default

Author: zvonimirfras

.storybook/preview.scss

@@ -1,7 +1,7 @@
 @import "~carbon-components/css/carbon-components.min.css";
 
 body {
-	margin: 20px;
+	padding: 20px;
 	// for tooltips/popovers ... can be on any chld element before the popover
 	// but body is most conveient
 	position: relative;

src/dialog/dialog-config.interface.ts

@@ -8,62 +8,37 @@ import { ElementRef, TemplateRef } from "@angular/core";
 export interface DialogConfig {
 	/**
 	 * Title for the `Dialog` header.
-	 * @type {string}
-	 * @memberof DialogConfig
 	 */
 	title: string;
 	/**
 	 * Body content for the `Dialog`.
-	 * @type {(string | TemplateRef<any>)}
-	 * @memberof DialogConfig
 	 */
 	content: string | TemplateRef<any>;
 	/**
 	 * Parameter for triggering `Dialog` display.
 	 * With the release of v2.0 the type will just be "click" or "hover".
-	 * @type {("click" | "hover" | "mouseenter")}
-	 * @memberof DialogConfig
 	 */
 	trigger: "click" | "hover" | "mouseenter";
 	/**
 	 * Parameter defining the placement in which the `Dialog` appears.
 	 * @type {Placement}
-	 * @memberof DialogConfig
 	 */
 	placement: string;
 	/**
 	 * Used to set the offset of the `Dialog` relative to the content it
 	 * is associated to.
-	 * @type {number}
-	 * @memberof DialogConfig
 	 */
 	gap: number;
 	/**
 	 * Reference to the Parent element that links the `Dialog`.
-	 * @type {ElementRef}
-	 * @memberof DialogConfig
 	 */
 	parentRef: ElementRef;
 	/**
-	 * Set to `true` will append `Dialog` to body causing
-	 * it to break out of containers.
-	 * @type {boolean}
-	 * @memberof DialogConfig
+	 * Set to `true` to open the dialog next to the triggering component
 	 */
-	appendToBody: boolean;
-	/**
-	 * Set to `true` will attempt to place
-	 * `Dialog` for maximum visibility.
-	 * TODO: remove - this doesn't actually do anything
-	 * @type {boolean | string}
-	 * @memberof DialogConfig
-	 * @deprecated
-	 */
-	autoPosition: boolean;
+	appendInline: boolean;
 	/**
 	 * Config object passed to the rendered component. (Optional)
-	 * @type {Object}
-	 * @memberof DialogConfig
 	 */
 	data: Object;
 	[propName: string]: any;

src/dialog/dialog.component.ts

@@ -166,7 +166,8 @@ export class Dialog implements OnInit, AfterViewInit, OnDestroy {
 
 		const placeDialogInContainer = () => {
 			// only do the work to find the scroll containers if we're appended to body
-			if (this.dialogConfig.appendToBody) {
+			// or skip this work if we're inline
+			if (!this.dialogConfig.appendInline) {
 				// walk the parents and subscribe to all the scroll events we can
 				while (node.parentElement && node !== document.body) {
 					if (isScrollableElement(node)) {
@@ -203,11 +204,11 @@ export class Dialog implements OnInit, AfterViewInit, OnDestroy {
 		// helper to find the position based on the current/given environment
 		const findPosition = (reference, target, placement) => {
 			let pos;
-			if (this.dialogConfig.appendToBody) {
+			if (this.dialogConfig.appendInline) {
+				pos = this.addGap[placement](position.findRelative(reference, target, placement));
+			} else {
 				pos = this.addGap[placement](position.findAbsolute(reference, target, placement));
 				pos = position.addOffset(pos, window.scrollY, window.scrollX);
-			} else {
-				pos = this.addGap[placement](position.findRelative(reference, target, placement));
 			}
 			return pos;
 		};

src/dialog/dialog.directive.ts

@@ -36,61 +36,55 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	/**
 	 * Title for the dialog
 	 * @type {string}
-	 * @memberof DialogDirective
 	 */
 	@Input() title = "";
 	/**
 	 * Dialog body content.
 	 * @type {(string | TemplateRef<any>)}
-	 * @memberof DialogDirective
 	 */
 	@Input() ibmDialog: string | TemplateRef<any>;
 	/**
 	 * Defines how the Dialog is triggered.(Hover and click behave the same on mobile - both respond to a single tap)
 	 * @type {("click" | "hover" | "mouseenter")}
-	 * @memberof DialogDirective
 	 */
 	@Input() trigger: "click" | "hover" | "mouseenter" = "click";
 	/**
 	 * Placement of the dialog, usually relative to the element the directive is on.
-	 * @memberof DialogDirective
 	 */
 	@Input() placement = "left";
 	/**
 	 * Class to add to the dialog container
 	 * @type {string}
-	 * @memberof DialogDirective
 	 */
 	@Input() wrapperClass: string;
 	/**
 	 * Spacing between the dialog and it's triggering element
 	 * @type {number}
-	 * @memberof DialogDirective
 	 */
 	@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)
-	 * @type {boolean}
-	 * @memberof DialogDirective
 	 */
-	@Input() appendToBody = false;
+	@Input() set appendToBody(v: boolean) {
+		console.log("`appendToBody` has been deprecated. Dialogs now append to the body by default.");
+		console.log("Ensure you have an `ibm-placeholder` in your app.");
+		console.log("Use `appendInline` if you need to position your dialogs within the normal page flow.");
+		this.appendInline = !v;
+	}
+	get appendToBody() {
+		return !this.appendInline;
+	}
 	/**
-	 * Determines if the Dialog will attempt to place itself for maximum visibility.
-	 * TODO: remove - this doesn't actually do anything
-	 * @type {boolean}
-	 * @memberof DialogDirective
-	 * @deprecated
+	 * Set to `true` to open the dialog next to the triggering component
 	 */
-	@Input() autoPosition: boolean;
+	@Input() appendInline = false;
 	/**
 	 * Optional data for templates
-	 * @memberof DialogDirective
 	 */
 	@Input() data = {};
 	/**
 	 * Config object passed to the rendered component
-	 * @type {DialogConfig}
-	 * @memberof DialogDirective
 	 */
 	@Output() onClose: EventEmitter<any> = new EventEmitter<any>();
 	dialogConfig: DialogConfig;
@@ -103,7 +97,6 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	 * @param {ElementRef} elementRef
 	 * @param {ViewContainerRef} viewContainerRef
 	 * @param {DialogService} dialogService
-	 * @memberof DialogDirective
 	 */
 	constructor(
 		protected elementRef: ElementRef,
@@ -113,7 +106,6 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	/**
 	 * Overrides 'touchstart' event to trigger a toggle on the Dialog.
 	 * @param {any} evt
-	 * @memberof DialogDirective
 	 */
 	@HostListener("touchstart", ["$event"])
 	onTouchStart(evt) {
@@ -131,8 +123,7 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 			parentRef: this.elementRef,
 			gap: this.gap,
 			trigger: this.trigger,
-			appendToBody: this.appendToBody,
-			autoPosition: this.autoPosition,
+			appendInline: this.appendInline,
 			wrapperClass: this.wrapperClass,
 			data: this.data
 		};
@@ -141,7 +132,6 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	/**
 	 * Sets the config object and binds events for hovering or clicking before
 	 * running code from child class.
-	 * @memberof DialogDirective
 	 */
 	ngOnInit() {
 		// fix for safari hijacking clicks
@@ -173,7 +163,6 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	/**
 	 * When the host dies, kill the popover.
 	 * - Useful for use in a modal or similar.
-	 * @memberof DialogDirective
 	 */
 	ngOnDestroy() {
 		this.close();
@@ -182,7 +171,6 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	/**
 	 * Helper method to call dialogService 'open'.
 	 * - Enforce accessibility by updating an aria attr for nativeElement.
-	 * @memberof DialogDirective
 	 */
 	open() {
 		this.dialogService.open(this.viewContainerRef, this.dialogConfig);
@@ -192,7 +180,6 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	/**
 	 * Helper method to call dialogService 'toggle'.
 	 * - Enforce accessibility by updating an aria attr for nativeElement.
-	 * @memberof DialogDirective
 	 */
 	toggle() {
 		this.dialogService.toggle(this.viewContainerRef, this.dialogConfig);
@@ -202,7 +189,6 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	/**
 	 * Helper method to call dialogService 'close'.
 	 * - Enforce accessibility by updating an aria attr for nativeElement.
-	 * @memberof DialogDirective
 	 */
 	close() {
 		this.dialogService.close(this.viewContainerRef);
@@ -213,7 +199,6 @@ export class DialogDirective implements OnInit, OnDestroy, OnChanges {
 	 * Empty method for child classes to override and specify additional init steps.
 	 * Run after DialogDirective completes it's ngOnInit.
 	 * @protected
-	 * @memberof DialogDirective
 	 */
 	protected onDialogInit() {}
 }

src/dialog/dialog.service.ts

@@ -103,13 +103,16 @@ export class DialogService {
 	 */
 	open(viewContainer: ViewContainerRef, dialogConfig: DialogConfig) {
 		if (!this.dialogRef) {
-			// holder for either the provided view, or the view from Placeholder
-			if (dialogConfig.appendToBody) {
-				// add our component to the placeholder
-				this.dialogRef = this.placeholderService.createComponent(this.componentFactory, this.injector);
-			} else {
+			if (dialogConfig.appendInline) {
 				// add our component to the view
 				this.dialogRef = viewContainer.createComponent(this.componentFactory, 0, this.injector);
+			} else if (!this.placeholderService.hasPlaceholderRef()) {
+				this.dialogRef = viewContainer.createComponent(this.componentFactory, 0, this.injector);
+				setTimeout(() => {
+					window.document.querySelector("body").appendChild(this.dialogRef.location.nativeElement);
+				});
+			} else {
+				this.dialogRef = this.placeholderService.createComponent(this.componentFactory, this.injector);
 			}
 
 			// initialize some extra options
@@ -140,7 +143,7 @@ export class DialogService {
 
 		if (this.dialogRef) {
 			let elementToFocus = this.dialogRef.instance.dialogConfig["previouslyFocusedElement"];
-			if (this.dialogRef.instance.dialogConfig.appendToBody) {
+			if (this.placeholderService.hasPlaceholderRef() && !this.dialogRef.instance.dialogConfig.appendInline) {
 				this.placeholderService.destroyComponent(this.dialogRef);
 			} else {
 				viewContainer.remove(viewContainer.indexOf(this.dialogRef.hostView));

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

@@ -17,7 +17,6 @@ import { I18n } from "./../../i18n/i18n.module";
 	template: `
 		<div
 			[ibmOverflowMenu]="options"
-			[appendToBody]="true"
 			[ngClass]="{'bx--overflow-menu--open': open === true}"
 			[attr.aria-label]="buttonLabel"
 			class="bx--overflow-menu"

src/dialog/tooltip/tooltip.directive.spec.ts

@@ -39,7 +39,10 @@ describe("Tooltip directive", () => {
 	it("should create the tooltip component and tooltip should appear at the top", () => {
 		TestBed.overrideComponent(TooltipTest, {
 			set: {
-				template: "<button ibmTooltip='Hello There' placement='top'>Me</button>"
+				template: `
+					<button ibmTooltip="Hello There" placement="top">Me</button>
+					<ibm-placeholder></ibm-placeholder>
+				`
 			}
 		});
 		const fixture = TestBed.createComponent(TooltipTest);