Fix modal event propagation (#8050)

* Fix event propagation

* Remove unneeded onkeyup method

* Move event handling code into SQL classes

src/sql/base/browser/ui/checkbox/checkbox.component.ts

@@ -8,7 +8,7 @@ import {
 	Output, OnChanges, SimpleChanges, EventEmitter
 } from '@angular/core';
 
-import { Checkbox as vsCheckbox } from 'sql/base/browser/ui/checkbox/checkbox';
+import { Checkbox as sqlCheckbox } from 'sql/base/browser/ui/checkbox/checkbox';
 
 @Component({
 	selector: 'checkbox',
@@ -22,14 +22,14 @@ export class Checkbox implements OnInit, OnChanges {
 
 	@Output() onChange = new EventEmitter<boolean>();
 
-	private _checkbox: vsCheckbox;
+	private _checkbox: sqlCheckbox;
 
 	constructor(
 		@Inject(forwardRef(() => ElementRef)) private _el: ElementRef
 	) { }
 
 	ngOnInit(): void {
-		this._checkbox = new vsCheckbox(this._el.nativeElement, {
+		this._checkbox = new sqlCheckbox(this._el.nativeElement, {
 			label: this.label,
 			ariaLabel: this.ariaLabel || this.label,
 			checked: this.checked,

src/sql/base/browser/ui/selectBox/selectBox.ts

@@ -13,6 +13,9 @@ import { IMessage, MessageType } from 'vs/base/browser/ui/inputbox/inputBox';
 import * as aria from 'vs/base/browser/ui/aria/aria';
 import * as nls from 'vs/nls';
 import { renderFormattedText, renderText, FormattedTextRenderOptions } from 'vs/base/browser/formattedTextRenderer';
+import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
+import { KeyCode } from 'vs/base/common/keyCodes';
+import { SelectBoxList } from 'vs/base/browser/ui/selectBox/selectBoxCustom';
 
 const $ = dom.$;
 
@@ -91,6 +94,23 @@ export class SelectBox extends vsSelectBox {
 		this._register(focusTracker);
 		this._register(focusTracker.onDidBlur(() => this._hideMessage()));
 		this._register(focusTracker.onDidFocus(() => this._showMessage()));
+		// Stop propagation - we've handled the event already and letting it bubble up causes issues with parent
+		// controls handling it (such as dialog pages)
+		this.onkeydown(this.selectElement, (e: IKeyboardEvent) => {
+			if (e.keyCode === KeyCode.Enter) {
+				dom.EventHelper.stop(e, true);
+			}
+		});
+		if (this.selectBoxDelegate instanceof SelectBoxList) {
+			// SelectBoxList uses its own custom drop down list so we need to also stop propagation from that or it'll
+			// also bubble up
+			this.onkeydown(this.selectBoxDelegate.selectDropDownContainer, (e: IKeyboardEvent) => {
+				if (e.keyCode === KeyCode.Enter) {
+					dom.EventHelper.stop(e, true);
+				}
+			});
+		}
+
 	}
 
 

src/sql/base/parts/editableDropdown/browser/dropdownTree.ts

@@ -9,6 +9,7 @@ import { generateUuid } from 'vs/base/common/uuid';
 import * as DOM from 'vs/base/browser/dom';
 import { Event, Emitter } from 'vs/base/common/event';
 import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
+import { KeyCode } from 'vs/base/common/keyCodes';
 
 export interface Template {
 	label: HTMLElement;
@@ -124,10 +125,24 @@ export class DropdownController extends TreeDefaults.DefaultController {
 		return response;
 	}
 
+	public onKeyDown(tree: tree.ITree, event: IKeyboardEvent): boolean {
+		// The enter key press is handled on key up by our base class (DefaultController) but
+		// we want to stop it here because we know we're going to handle it (by selecting the item)
+		// and letting it propagate up means that other controls may incorrectly handle it first
+		// if they're listening to onKeyDown
+		const response = super.onKeyDown(tree, event);
+		if (event.keyCode === KeyCode.Enter) {
+			DOM.EventHelper.stop(event, true);
+			return true;
+		}
+		return response;
+	}
+
 	protected onEnter(tree: tree.ITree, event: IKeyboardEvent): boolean {
 		let response = super.onEnter(tree, event);
 		if (response) {
 			this._onSelectionChange.fire(tree.getSelection()[0]);
+			DOM.EventHelper.stop(event, true);
 		}
 		return response;
 	}

src/sql/workbench/browser/modelComponents/card.component.ts

@@ -16,6 +16,9 @@ import { IColorTheme, IWorkbenchThemeService } from 'vs/workbench/services/theme
 import { ComponentWithIconBase } from 'sql/workbench/browser/modelComponents/componentWithIconBase';
 import { IComponent, IComponentDescriptor, IModelStore, ComponentEventType } from 'sql/workbench/browser/modelComponents/interfaces';
 import { StatusIndicator, CardProperties, ActionDescriptor, CardDescriptionItem } from 'sql/workbench/api/common/sqlExtHostTypes';
+import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
+import { KeyCode } from 'vs/base/common/keyCodes';
+import * as DOM from 'vs/base/browser/dom';
 
 @Component({
 	templateUrl: decodeURI(require.toUrl('./card.component.html'))
@@ -37,6 +40,12 @@ export default class CardComponent extends ComponentWithIconBase implements ICom
 		this.baseInit();
 		this._register(this.themeService.onDidColorThemeChange(this.updateTheme, this));
 		this.updateTheme(this.themeService.getColorTheme());
+		this.onkeydown(this._el.nativeElement, (e: StandardKeyboardEvent) => {
+			if (e.keyCode === KeyCode.Enter) {
+				this.onCardClick();
+				DOM.EventHelper.stop(e, true);
+			}
+		});
 
 	}
 

src/sql/workbench/browser/modelComponents/inputbox.component.ts

@@ -22,6 +22,7 @@ import * as nls from 'vs/nls';
 import { inputBackground, inputBorder } from 'vs/platform/theme/common/colorRegistry';
 import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { KeyCode } from 'vs/base/common/keyCodes';
+import * as DOM from 'vs/base/browser/dom';
 
 @Component({
 	selector: 'modelview-inputBox',
@@ -78,7 +79,7 @@ export default class InputBoxComponent extends ComponentBase implements ICompone
 						args: this._input.value
 					});
 					if (this.stopEnterPropagation) {
-						e.stopPropagation();
+						DOM.EventHelper.stop(e, true);
 					}
 				}
 			});
@@ -89,7 +90,7 @@ export default class InputBoxComponent extends ComponentBase implements ICompone
 			this._textAreaInput = new InputBox(this._textareaContainer.nativeElement, this.contextViewService, textAreaInputOptions);
 			this.onkeydown(this._textAreaInput.inputElement, (e: StandardKeyboardEvent) => {
 				if (this.tryHandleKeyEvent(e)) {
-					e.stopPropagation();
+					DOM.EventHelper.stop(e, true);
 				}
 				if (e.keyCode === KeyCode.Enter) {
 					this.fireEvent({
@@ -97,7 +98,7 @@ export default class InputBoxComponent extends ComponentBase implements ICompone
 						args: this._textAreaInput.value
 					});
 					if (this.stopEnterPropagation) {
-						e.stopPropagation();
+						DOM.EventHelper.stop(e, true);
 					}
 				}
 				// Else assume that keybinding service handles routing this to a command

src/sql/workbench/browser/modelComponents/tree.component.ts

@@ -24,7 +24,7 @@ import { DefaultFilter, DefaultAccessibilityProvider, DefaultController } from '
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { ITreeComponentItem } from 'sql/workbench/common/views';
 import { TreeViewDataProvider } from 'sql/workbench/browser/modelComponents/treeViewDataProvider';
-import { getContentHeight, getContentWidth } from 'vs/base/browser/dom';
+import * as DOM from 'vs/base/browser/dom';
 import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { KeyCode } from 'vs/base/common/keyCodes';
 
@@ -127,7 +127,7 @@ export default class TreeComponent extends ComponentBase implements IComponent,
 				// This might have unintended effects such as a dialog closing.
 				if (e.keyCode === KeyCode.Enter) {
 					this._tree.toggleExpansion(this._tree.getFocus());
-					e.stopPropagation();
+					DOM.EventHelper.stop(e, true);
 				}
 			});
 			this._tree.refresh();
@@ -149,8 +149,8 @@ export default class TreeComponent extends ComponentBase implements IComponent,
 		let width: number = this.convertSizeToNumber(this.width);
 		let height: number = this.convertSizeToNumber(this.height);
 		this._tree.layout(
-			height && height > 0 ? height : getContentHeight(this._inputContainer.nativeElement),
-			width && width > 0 ? width : getContentWidth(this._inputContainer.nativeElement));
+			height && height > 0 ? height : DOM.getContentHeight(this._inputContainer.nativeElement),
+			width && width > 0 ? width : DOM.getContentWidth(this._inputContainer.nativeElement));
 	}
 
 	public setLayout(layout: any): void {