From 6ab5504e2debf9408883256331ee28bb6f52032f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Oct 2019 16:51:50 -0700 Subject: [PATCH 1/7] Prelimnary fix --- .../react-common/monacoEditor.tsx | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index 35f5c1c778f4..92d384174f8e 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -59,6 +59,7 @@ export class MonacoEditor extends React.Component { + console.log('Blurred'); + console.log('Blurred'); + console.log('Blurred'); + console.log('Blurred'); + console.log('Blurred'); + this.hideParameterHints(); + })); + // Track focus changes to make sure we update our widget parent and widget position this.subscriptions.push(editor.onDidFocusEditorWidget(() => { this.throttledUpdateWidgetPosition(); @@ -434,7 +444,80 @@ export class MonacoEditor extends React.Component this.hideParameterHints(), 100); + } + } + private hideParameterHints(){ + if (!this.state.editor || !this.state.editor.getDomNode() || !this.widgetParent){ + return; + } + const editorDom = this.state.editor.getDomNode()!; + const elements = document.querySelectorAll(':hover'); + const visibleParameterHintsWidgets = this.widgetParent.querySelectorAll('.parameter-hints-widget.visible'); + if (elements.length === 0 && visibleParameterHintsWidgets.length === 0){ + return; + } + if (elements.length === 0 && visibleParameterHintsWidgets.length > 0){ + return; + } + const knownParameterHintsWidgets = this.widgetParent.querySelectorAll('.parameter-hints-widget'); + + let isHoveringOverParameterWidget = false; + // Lets not assume we'll have the exact same DOM for parameter widgets. + // So, just remove the event handler. + if (this.parameterWidget){ + this.parameterWidget.removeEventListener('mouseleave', this.outermostParentLeave); + } + const parameterWidgetClasses = ['editor-widget', 'parameter-hints-widget', 'visible']; + + elements.forEach(item => { + if (!item || !item.className) { + return; + } + const classes = item.className.split(' '); + if (!parameterWidgetClasses.every(cls => classes.indexOf(cls) >= 0)){ + // Not all classes required in a parameter hint widget are in this element. + return; + } + // Check if the parameter hints widget currently displayed belongs to this monaco editor. + let widgetBelongsToThisEditor = false; + knownParameterHintsWidgets.forEach(widget => { + if (widget === item){ + widgetBelongsToThisEditor = true; + } + }); + if (!widgetBelongsToThisEditor){ + return; + } + this.parameterWidget = item; + isHoveringOverParameterWidget = true; + }); + if (isHoveringOverParameterWidget && this.parameterWidget){ + this.parameterWidget.addEventListener('mouseleave', this.outermostParentLeave); + // Check if the user has move away from this cell + // In which case we'll need to hide the parameter hints. + setTimeout(() => this.hideParameterHints(), 1000); + return; } + if (!isHoveringOverParameterWidget && visibleParameterHintsWidgets.length === 0){ + return; + } + + // Hitting escape on the monaco editor when it doesn't have focus doesn't seem to work. + // Hence we need to hide it manually (as well). + knownParameterHintsWidgets.forEach(widget => { + widget.setAttribute('class', widget.className.split(' ').filter(cls => cls !== 'visible').join(' ')); + // tslint:disable-next-line: no-any + const ele = widget as any; + if (ele.hasOwnProperty('style') && ele.style.hasOwnProperty('visibility')) { + ele.style.visibility = 'hidden'; + } + }); + // tslint:disable-next-line: no-any + const eventOptions = {key: 'Escape', code: 27, keyCode: 27, bubbles: true, cancelable: true } as any; + editorDom.dispatchEvent(new KeyboardEvent('keydown', eventOptions)); } private updateMargin(editor: monacoEditor.editor.IStandaloneCodeEditor) { From 305e57205c91760d374ff579e26778edf73cc9a8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 14 Oct 2019 17:51:05 -0700 Subject: [PATCH 2/7] More comments and fine tune --- .../react-common/monacoEditor.tsx | 128 +++++++++++------- 1 file changed, 80 insertions(+), 48 deletions(-) diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index 92d384174f8e..415d3aa8407f 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -59,6 +59,14 @@ export class MonacoEditor extends React.Component { - console.log('Blurred'); - console.log('Blurred'); - console.log('Blurred'); - console.log('Blurred'); - console.log('Blurred'); - this.hideParameterHints(); + this.hideParameterWidget(); })); // Track focus changes to make sure we update our widget parent and widget position @@ -202,7 +206,10 @@ export class MonacoEditor extends React.Component this.hideParameterHints(), 100); + setTimeout(() => this.hideParameterWidget(), 100); } } - private hideParameterHints(){ + + /** + * This will hide the parameter widget if the user is not hovering over + * the parameter widget for this monaco editor. + * + * @private + * @returns + * @memberof MonacoEditor + */ + private hideParameterWidget(){ if (!this.state.editor || !this.state.editor.getDomNode() || !this.widgetParent){ return; } - const editorDom = this.state.editor.getDomNode()!; - const elements = document.querySelectorAll(':hover'); - const visibleParameterHintsWidgets = this.widgetParent.querySelectorAll('.parameter-hints-widget.visible'); - if (elements.length === 0 && visibleParameterHintsWidgets.length === 0){ + // Find all elements that the user is hovering over. + // Its possible the parameter widget is one of them. + const hoverElements: Element[] = Array.prototype.slice.call(document.querySelectorAll(':hover')); + // Find all parameter widgets related to this monaco editor that are currently displayed. + const visibleParameterHintsWidgets: Element[] = Array.prototype.slice.call(this.widgetParent.querySelectorAll('.parameter-hints-widget.visible')); + if (hoverElements.length === 0 && visibleParameterHintsWidgets.length === 0){ return; } - if (elements.length === 0 && visibleParameterHintsWidgets.length > 0){ - return; - } - const knownParameterHintsWidgets = this.widgetParent.querySelectorAll('.parameter-hints-widget'); - let isHoveringOverParameterWidget = false; + // Find all parameter widgets related to this monaco editor. + const knownParameterHintsWidgets: HTMLDivElement[] = Array.prototype.slice.call(this.widgetParent.querySelectorAll('.parameter-hints-widget')); + // Lets not assume we'll have the exact same DOM for parameter widgets. - // So, just remove the event handler. + // So, just remove the event handler, and add it again later. if (this.parameterWidget){ this.parameterWidget.removeEventListener('mouseleave', this.outermostParentLeave); } + // These are the classes that will appear on a parameter widget when they are visible. const parameterWidgetClasses = ['editor-widget', 'parameter-hints-widget', 'visible']; - elements.forEach(item => { - if (!item || !item.className) { - return; + // Find the parameter widget the user is currently hovering over. + this.parameterWidget = hoverElements.find(item => { + if (!item.className) { + return false; } + // Check if user is hovering over a parameter widget. const classes = item.className.split(' '); if (!parameterWidgetClasses.every(cls => classes.indexOf(cls) >= 0)){ // Not all classes required in a parameter hint widget are in this element. - return; - } - // Check if the parameter hints widget currently displayed belongs to this monaco editor. - let widgetBelongsToThisEditor = false; - knownParameterHintsWidgets.forEach(widget => { - if (widget === item){ - widgetBelongsToThisEditor = true; - } - }); - if (!widgetBelongsToThisEditor){ - return; + // Hence this is not a parameter widget. + return false; } - this.parameterWidget = item; - isHoveringOverParameterWidget = true; + + // Ok, this element that the user is hovering over is a parameter widget. + + // Next, check whether this parameter widget belongs to this monaco editor. + // We have a list of parameter widgets that belong to this editor, hence a simple lookup. + return knownParameterHintsWidgets.some(widget => widget === item); }); - if (isHoveringOverParameterWidget && this.parameterWidget){ + + if (this.parameterWidget){ + // We know the user is hovering over the parameter widget for this editor. + // Hovering could mean the user is scrolling through a large parameter list. + // We need to add a mouse leave event handler, so as to hide this. this.parameterWidget.addEventListener('mouseleave', this.outermostParentLeave); - // Check if the user has move away from this cell - // In which case we'll need to hide the parameter hints. - setTimeout(() => this.hideParameterHints(), 1000); + + // In case the event handler doesn't get fired, have a backup of checking within 1s. + setTimeout(() => this.hideParameterWidget(), 1000); + return; + } + if (visibleParameterHintsWidgets.length === 0){ + // There are no parameter widgets displayed for this editor. + // Hence nothing to do. return; } - if (!isHoveringOverParameterWidget && visibleParameterHintsWidgets.length === 0){ + // If the editor has focus, don't hide the parameter widget. + // This is the default behavior. Let the user hit `Escape` or click somewhere + // to forcefully hide the parameter widget. + if (this.state.editor.hasWidgetFocus()) { return; } - // Hitting escape on the monaco editor when it doesn't have focus doesn't seem to work. + // If we got here, then the user is not hovering over the paramter widgets. + // & the editor doesn't have focus. + // However some of the parameter widgets associated with this monaco editor are visible. + // We need to hide them. + + // Solution 1: Send the `Escape` key stroke to monaco, this will hide the parameters widget. + // tslint:disable-next-line: no-any + const eventOptions = {key: 'Escape', code: 27, keyCode: 27, bubbles: true, cancelable: true } as any; + const editorDom = this.state.editor.getDomNode()!; + editorDom.dispatchEvent(new KeyboardEvent('keydown', eventOptions)); + + // Solution 2: Hitting escape on the monaco editor when it doesn't have focus doesn't work. // Hence we need to hide it manually (as well). knownParameterHintsWidgets.forEach(widget => { widget.setAttribute('class', widget.className.split(' ').filter(cls => cls !== 'visible').join(' ')); - // tslint:disable-next-line: no-any - const ele = widget as any; - if (ele.hasOwnProperty('style') && ele.style.hasOwnProperty('visibility')) { - ele.style.visibility = 'hidden'; + if (widget.style.visibility !== 'hidden') { + widget.style.visibility = 'hidden'; } }); - // tslint:disable-next-line: no-any - const eventOptions = {key: 'Escape', code: 27, keyCode: 27, bubbles: true, cancelable: true } as any; - editorDom.dispatchEvent(new KeyboardEvent('keydown', eventOptions)); } private updateMargin(editor: monacoEditor.editor.IStandaloneCodeEditor) { From dde37cda7b6d58a1c813b768446139bb08522085 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 15 Oct 2019 10:28:12 -0700 Subject: [PATCH 3/7] Add comments --- src/datascience-ui/react-common/monacoEditor.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index 415d3aa8407f..0a999c27c495 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -475,6 +475,8 @@ export class MonacoEditor extends React.Component { widget.setAttribute('class', widget.className.split(' ').filter(cls => cls !== 'visible').join(' ')); From e7a7db6adc6669f8080e38438e09798b200e1c10 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 15 Oct 2019 10:32:21 -0700 Subject: [PATCH 4/7] HIde parameter widget when no longer required --- news/2 Fixes/7851.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/7851.md diff --git a/news/2 Fixes/7851.md b/news/2 Fixes/7851.md new file mode 100644 index 000000000000..d45e641a6d22 --- /dev/null +++ b/news/2 Fixes/7851.md @@ -0,0 +1 @@ +Hide the parameters intellisense widget in the `Notebook Editor` when it is not longer required. From e624f14fbf6c1f220929b94f040d57abd6fb112e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 15 Oct 2019 11:13:08 -0700 Subject: [PATCH 5/7] Comments --- src/datascience-ui/react-common/monacoEditor.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index 0a999c27c495..f5ac7e83f8f1 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -461,6 +461,8 @@ export class MonacoEditor extends React.Component Date: Tue, 15 Oct 2019 11:13:45 -0700 Subject: [PATCH 6/7] Comments --- src/datascience-ui/react-common/monacoEditor.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index f5ac7e83f8f1..e890161cf959 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -540,8 +540,7 @@ export class MonacoEditor extends React.Component { widget.setAttribute('class', widget.className.split(' ').filter(cls => cls !== 'visible').join(' ')); if (widget.style.visibility !== 'hidden') { From 0c3134718563c99c3c13e319422a1a57d3def22a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 15 Oct 2019 11:22:05 -0700 Subject: [PATCH 7/7] Added comments --- src/datascience-ui/react-common/monacoEditor.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index e890161cf959..4c2d19915c8f 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -462,6 +462,10 @@ export class MonacoEditor extends React.Component