From dff955ca805994bfd6c2480fd97245148018444d Mon Sep 17 00:00:00 2001 From: Colin Grant Date: Mon, 23 Aug 2021 18:24:26 +0200 Subject: [PATCH] Handle focus more parsimoniously Signed-off-by: Colin Grant --- .../breadcrumbs/breadcrumb-popup-container.ts | 25 +++---------------- .../breadcrumbs/breadcrumb-renderer.tsx | 8 +++--- .../filepath-breadcrumbs-contribution.ts | 1 + .../outline-breadcrumbs-contribution.tsx | 1 + 4 files changed, 10 insertions(+), 25 deletions(-) diff --git a/packages/core/src/browser/breadcrumbs/breadcrumb-popup-container.ts b/packages/core/src/browser/breadcrumbs/breadcrumb-popup-container.ts index bfe3eac3b591c..db1643a5c20d6 100644 --- a/packages/core/src/browser/breadcrumbs/breadcrumb-popup-container.ts +++ b/packages/core/src/browser/breadcrumbs/breadcrumb-popup-container.ts @@ -72,32 +72,15 @@ export class BreadcrumbPopupContainer implements Disposable { result.style.left = `${position.x}px`; result.style.top = `${position.y}px`; result.tabIndex = 0; - result.onblur = event => this.onBlur(event, this.breadcrumbId); + result.addEventListener('focusout', this.onFocusOut); this.parent.appendChild(result); return result; } - protected onBlur = (event: FocusEvent, breadcrumbId: string) => { - if (event.relatedTarget && event.relatedTarget instanceof HTMLElement) { - // event.relatedTarget is the element that has the focus after this popup looses the focus. - // If a breadcrumb was clicked the following holds the breadcrumb ID of the clicked breadcrumb. - const clickedBreadcrumbId = event.relatedTarget.getAttribute('data-breadcrumb-id'); - if (clickedBreadcrumbId && clickedBreadcrumbId === breadcrumbId) { - // This is a click on the breadcrumb that has openend this popup. - // We do not close this popup here but let the click event of the breadcrumb handle this instead - // because it needs to know that this popup is open to decide if it just closes this popup or - // also opens a new popup. - return; - } - if (this._container.contains(event.relatedTarget)) { - // A child element gets focus. Set the focus to the container again. - // Otherwise the popup would not be closed when elements outside the popup get the focus. - // A popup content should not rely on getting a focus. - this._container.focus(); - return; - } + protected onFocusOut = (event: FocusEvent) => { + if (!(event.relatedTarget instanceof Element) || !this._container.contains(event.relatedTarget)) { + this.dispose(); } - this.dispose(); }; protected escFunction = (event: KeyboardEvent) => { diff --git a/packages/core/src/browser/breadcrumbs/breadcrumb-renderer.tsx b/packages/core/src/browser/breadcrumbs/breadcrumb-renderer.tsx index 6385e8c8f6244..3488fdb19cd80 100644 --- a/packages/core/src/browser/breadcrumbs/breadcrumb-renderer.tsx +++ b/packages/core/src/browser/breadcrumbs/breadcrumb-renderer.tsx @@ -24,15 +24,15 @@ export interface BreadcrumbRenderer { /** * Renders the given breadcrumb. If `onClick` is given, it is called on breadcrumb click. */ - render(breadcrumb: Breadcrumb, onClick?: (breadcrumb: Breadcrumb, event: React.MouseEvent) => void): React.ReactNode; + render(breadcrumb: Breadcrumb, onMouseDown?: (breadcrumb: Breadcrumb, event: React.MouseEvent) => void): React.ReactNode; } @injectable() export class DefaultBreadcrumbRenderer implements BreadcrumbRenderer { - render(breadcrumb: Breadcrumb, onClick?: (breadcrumb: Breadcrumb, event: React.MouseEvent) => void): React.ReactNode { + render(breadcrumb: Breadcrumb, onMouseDown?: (breadcrumb: Breadcrumb, event: React.MouseEvent) => void): React.ReactNode { return
  • onClick && onClick(breadcrumb, event)} + className={Breadcrumbs.Styles.BREADCRUMB_ITEM + (!onMouseDown ? '' : ' ' + Breadcrumbs.Styles.BREADCRUMB_ITEM_HAS_POPUP)} + onMouseDown={event => onMouseDown && onMouseDown(breadcrumb, event)} tabIndex={0} data-breadcrumb-id={breadcrumb.id} > diff --git a/packages/filesystem/src/browser/breadcrumbs/filepath-breadcrumbs-contribution.ts b/packages/filesystem/src/browser/breadcrumbs/filepath-breadcrumbs-contribution.ts index 85f45c24f8f42..da8b15471c8a3 100644 --- a/packages/filesystem/src/browser/breadcrumbs/filepath-breadcrumbs-contribution.ts +++ b/packages/filesystem/src/browser/breadcrumbs/filepath-breadcrumbs-contribution.ts @@ -80,6 +80,7 @@ export class FilepathBreadcrumbsContribution implements BreadcrumbsContribution if (targetNode && SelectableTreeNode.is(targetNode)) { model.selectNode(targetNode); } + this.breadcrumbsFileTreeWidget.activate(); } }); return { diff --git a/packages/outline-view/src/browser/outline-breadcrumbs-contribution.tsx b/packages/outline-view/src/browser/outline-breadcrumbs-contribution.tsx index 6ec014db02014..f6f9e966d4b57 100644 --- a/packages/outline-view/src/browser/outline-breadcrumbs-contribution.tsx +++ b/packages/outline-view/src/browser/outline-breadcrumbs-contribution.tsx @@ -137,6 +137,7 @@ export class OutlineBreadcrumbsContribution implements BreadcrumbsContribution { this.outlineView.model.selectNode(node); this.outlineView.model.collapseAll(); Widget.attach(this.outlineView, parent); + this.outlineView.activate(); toDisposeOnHide.pushAll([ this.outlineView.model.onExpansionChanged(expandedNode => SelectableTreeNode.is(expandedNode) && this.outlineView.model.selectNode(expandedNode)), Disposable.create(() => {