From 5db6857c49ce98a74b93933cecd2273680f50888 Mon Sep 17 00:00:00 2001 From: Alan Ren Date: Thu, 25 Mar 2021 09:49:16 -0700 Subject: [PATCH] only return visible elements as focusable (#14864) --- src/sql/base/browser/dom.ts | 20 ++++++++++ src/sql/workbench/browser/modal/modal.ts | 51 +++++++++++------------- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/sql/base/browser/dom.ts b/src/sql/base/browser/dom.ts index 91a99d36ba..c2483cfc0e 100644 --- a/src/sql/base/browser/dom.ts +++ b/src/sql/base/browser/dom.ts @@ -44,3 +44,23 @@ export function convertSizeToNumber(size: number | string | undefined): number { } return +size; } + +const tabbableElementsQuerySelector = 'a[href], area[href], input:not([disabled]), select:not([disabled]), textarea:not([disabled]), button:not([disabled]), [tabindex="0"]'; + +/** + * Get the focusable elements inside a HTML element + * @param container The container element inside which we should look for the focusable elements + * @returns The focusable elements + */ +export function getFocusableElements(container: HTMLElement): HTMLElement[] { + const elements = []; + container.querySelectorAll(tabbableElementsQuerySelector).forEach((element: HTMLElement) => { + const style = window.getComputedStyle(element); + // We should only return the elements that are visible. There are many ways to hide an element, for example setting the + // visibility attribute to hidden/collapse, setting the display property to none, or if one of its ancestors is invisible. + if (element.offsetWidth > 0 && element.offsetHeight > 0 && style.visibility === 'visible') { + elements.push(element); + } + }); + return elements; +} diff --git a/src/sql/workbench/browser/modal/modal.ts b/src/sql/workbench/browser/modal/modal.ts index 513de71281..3efc97a79c 100644 --- a/src/sql/workbench/browser/modal/modal.ts +++ b/src/sql/workbench/browser/modal/modal.ts @@ -2,30 +2,32 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import 'vs/css!./media/modal'; + import 'vs/css!./media/calloutDialog'; -import { attachButtonStyler } from 'vs/platform/theme/common/styler'; -import { Color } from 'vs/base/common/color'; -import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; -import { mixin } from 'vs/base/common/objects'; -import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; +import 'vs/css!./media/modal'; +import { getFocusableElements } from 'sql/base/browser/dom'; +import { Button } from 'sql/base/browser/ui/button/button'; +import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry'; +import * as TelemetryKeys from 'sql/platform/telemetry/common/telemetryKeys'; import * as DOM from 'vs/base/browser/dom'; import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent'; -import { generateUuid } from 'vs/base/common/uuid'; -import { IContextKeyService, RawContextKey, IContextKey } from 'vs/platform/contextkey/common/contextkey'; -import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; -import { Button } from 'sql/base/browser/ui/button/button'; -import * as TelemetryKeys from 'sql/platform/telemetry/common/telemetryKeys'; -import { localize } from 'vs/nls'; -import { isUndefinedOrNull } from 'vs/base/common/types'; -import { ILogService } from 'vs/platform/log/common/log'; -import { ITextResourcePropertiesService } from 'vs/editor/common/services/textResourceConfigurationService'; -import { URI } from 'vs/base/common/uri'; -import { Schemas } from 'vs/base/common/network'; -import { IThemable } from 'vs/base/common/styler'; -import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry'; -import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; import { alert } from 'vs/base/browser/ui/aria/aria'; +import { Color } from 'vs/base/common/color'; +import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; +import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; +import { Schemas } from 'vs/base/common/network'; +import { mixin } from 'vs/base/common/objects'; +import { IThemable } from 'vs/base/common/styler'; +import { isUndefinedOrNull } from 'vs/base/common/types'; +import { URI } from 'vs/base/common/uri'; +import { generateUuid } from 'vs/base/common/uuid'; +import { ITextResourcePropertiesService } from 'vs/editor/common/services/textResourceConfigurationService'; +import { localize } from 'vs/nls'; +import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; +import { IContextKey, IContextKeyService, RawContextKey } from 'vs/platform/contextkey/common/contextkey'; +import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; +import { ILogService } from 'vs/platform/log/common/log'; +import { attachButtonStyler } from 'vs/platform/theme/common/styler'; import { IThemeService } from 'vs/platform/theme/common/themeService'; export enum MessageLevel { @@ -97,8 +99,6 @@ const defaultOptions: IModalOptions = { dialogProperties: undefined }; -const tabbableElementsQuerySelector = 'a[href], area[href], input:not([disabled]), select:not([disabled]), textarea:not([disabled]), button:not([disabled]), [tabindex="0"]'; - export type HideReason = 'close' | 'cancel' | 'ok'; export abstract class Modal extends Disposable implements IThemable { @@ -400,7 +400,7 @@ export abstract class Modal extends Disposable implements IThemable { * Figures out the first and last elements which the user can tab to in the dialog */ public setFirstLastTabbableElement() { - const tabbableElements = this._bodyContainer!.querySelectorAll(tabbableElementsQuerySelector); + const tabbableElements = getFocusableElements(this._bodyContainer!); if (tabbableElements && tabbableElements.length > 0) { this._firstTabbableElement = tabbableElements[0]; this._lastTabbableElement = tabbableElements[tabbableElements.length - 1]; @@ -413,16 +413,13 @@ export abstract class Modal extends Disposable implements IThemable { public setInitialFocusedElement() { // Try to find focusable element in dialog pane rather than overall container. _modalBodySection contains items in the pane for a wizard. // This ensures that we are setting the focus on a useful element in the form when possible. - const focusableElements = this._modalBodySection ? - this._modalBodySection.querySelectorAll(tabbableElementsQuerySelector) : - this._bodyContainer!.querySelectorAll(tabbableElementsQuerySelector); + const focusableElements = getFocusableElements(this._modalBodySection ?? this._bodyContainer!); if (focusableElements && focusableElements.length > 0) { (focusableElements[0]).focus(); } } - /** * Tasks to perform before callout dialog is shown * Includes: positioning of dialog