From 7ef96a56f83be44874e88113ce57ee580259ef55 Mon Sep 17 00:00:00 2001 From: "michael.kuss" Date: Wed, 21 Jul 2021 08:35:49 +0000 Subject: [PATCH] fix(chart): Fixed chart selection area focus. --- .../chart/src/chart-focus-anchor.spec.ts | 118 +++++++++++ .../chart/src/chart-focus-anchor.ts | 187 ++++++++++++++++++ .../chart/src/chart-module.ts | 3 + libs/barista-components/chart/src/chart.ts | 3 + .../chart/src/range/range.html | 21 +- .../chart/src/range/range.spec.ts | 7 + .../chart/src/range/range.ts | 8 - .../src/selection-area/selection-area.spec.ts | 16 -- .../src/selection-area/selection-area.ts | 46 ----- .../chart/src/timestamp/timestamp.html | 2 - .../chart/src/timestamp/timestamp.ts | 9 - libs/barista-components/chart/src/utils.ts | 53 ----- 12 files changed, 337 insertions(+), 136 deletions(-) create mode 100644 libs/barista-components/chart/src/chart-focus-anchor.spec.ts create mode 100644 libs/barista-components/chart/src/chart-focus-anchor.ts diff --git a/libs/barista-components/chart/src/chart-focus-anchor.spec.ts b/libs/barista-components/chart/src/chart-focus-anchor.spec.ts new file mode 100644 index 0000000000..2d585bab06 --- /dev/null +++ b/libs/barista-components/chart/src/chart-focus-anchor.spec.ts @@ -0,0 +1,118 @@ +/** + * @license + * Copyright 2021 Dynatrace LLC + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// tslint:disable no-lifecycle-call no-use-before-declare no-magic-numbers +// tslint:disable no-any max-file-line-count no-unbound-method use-component-selector + +import { InteractivityChecker } from '@angular/cdk/a11y'; +import { Platform } from '@angular/cdk/platform'; +import { CommonModule } from '@angular/common'; +import { Component, Provider } from '@angular/core'; +import { TestBed, waitForAsync } from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; +import { createComponent } from '@dynatrace/testing/browser'; +import { DtChart } from './chart'; +import { DtChartFocusAnchor, DtChartFocusTarget } from './chart-focus-anchor'; + +/** Mock Chart so the targets have a place to register themselves */ +export class MockChart { + _focusTargets = new Set(); +} + +/** + * Overrides the default InteractivityChecker to alway set the + * ignoreVisibility flag to true, as all DOM nodes in Jest would + * otherwise be detected as not visible and therefore not focusalbe. + */ +export class TestInteractivityChecker extends InteractivityChecker { + isFocusable(element: any): boolean { + return super.isFocusable(element, { ignoreVisibility: true }); + } +} + +export const TEST_INTERACTIVITY_CHECKER_PROVIDER: Provider = { + provide: InteractivityChecker, + useClass: TestInteractivityChecker, + deps: [Platform], +}; + +describe('DtChartFocusAnchor', () => { + beforeEach( + waitForAsync(() => { + TestBed.configureTestingModule({ + imports: [CommonModule], + providers: [ + { provide: DtChart, useClass: MockChart }, + TEST_INTERACTIVITY_CHECKER_PROVIDER, + ], + declarations: [ + DtChartFocusAnchor, + DtChartFocusTarget, + TestAppSkipFocusElement, + TestAppFindFocusElement, + ], + }); + + TestBed.compileComponents(); + }), + ); + + it('should shift focus to the specified target', () => { + const fixture = createComponent(TestAppSkipFocusElement); + const focusAnchor = fixture.debugElement.query( + By.css('dt-chart-focus-anchor'), + ); + const buttonDebugElement = fixture.debugElement.query( + By.css('[dtChartFocusTarget]'), + ); + + focusAnchor.nativeElement.focus(); + fixture.detectChanges(); + expect(document.activeElement).toBe(buttonDebugElement.nativeElement); + }); + + it('should shift focus to the specified target', () => { + const fixture = createComponent(TestAppFindFocusElement); + const focusAnchor = fixture.debugElement.query( + By.css('dt-chart-focus-anchor'), + ); + const buttonDebugElement = fixture.debugElement.query(By.css('button')); + + focusAnchor.nativeElement.focus(); + fixture.detectChanges(); + expect(document.activeElement).toBe(buttonDebugElement.nativeElement); + }); +}); + +@Component({ + selector: 'test-chart-without-selection-area', + template: ` + + + + `, +}) +export class TestAppSkipFocusElement {} + +@Component({ + selector: 'test-chart-without-selection-area', + template: ` + +
Should be skipped
+ + `, +}) +export class TestAppFindFocusElement {} diff --git a/libs/barista-components/chart/src/chart-focus-anchor.ts b/libs/barista-components/chart/src/chart-focus-anchor.ts new file mode 100644 index 0000000000..990a8d2844 --- /dev/null +++ b/libs/barista-components/chart/src/chart-focus-anchor.ts @@ -0,0 +1,187 @@ +/** + * @license + * Copyright 2021 Dynatrace LLC + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { InteractivityChecker } from '@angular/cdk/a11y'; +import { TAB } from '@angular/cdk/keycodes'; +import { Directive, ElementRef, Input, OnDestroy } from '@angular/core'; +import { DtChart } from './chart'; +import { _readKeyCode } from '@dynatrace/barista-components/core'; + +type NativeFocusTarget = Element & { focus: () => void }; + +/** Focus event of the DtChartFocusAnchor. */ +export interface DtChartFocusAnchorFocusEvent { + nativeEvt: FocusEvent; + anchor: DtChartFocusAnchor; +} + +/** Element that redirects focus when received to a next or previous target depending on the tab-direction. */ +@Directive({ + selector: 'dt-chart-focus-anchor', + host: { + class: 'cdk-visually-hidden', + tabindex: '0', + 'aria-hidden': 'true', + '(focus)': '_handleFocus($event)', + '(keydown)': '_handleKeyUp($event)', + }, +}) +export class DtChartFocusAnchor { + /** The next target to shift focus to. */ + @Input() nextTarget: string | undefined; + + /** The previous target to shift focus to. */ + @Input() prevTarget: string | undefined; + + constructor( + private _chart: DtChart, + private _elementRef: ElementRef, + private _checker: InteractivityChecker, + ) {} + + /** @internal Handle keyup event, capture tab & shift+tab key-presses. */ + _handleKeyUp(event: KeyboardEvent): void { + const { nextTarget, prevTarget } = this._findTargets(); + if (_readKeyCode(event) === TAB) { + if (event.shiftKey) { + this._focusTarget(prevTarget, event); + } else { + this._focusTarget(nextTarget, event); + } + } + } + + /** @internal Handle receiving focus. */ + _handleFocus(event: FocusEvent): void { + const { nextTarget, prevTarget } = this._findTargets(); + + if (event.relatedTarget === nextTarget && prevTarget) { + // shift + tab + this._focusTarget(prevTarget, event); + } else if (nextTarget) { + // tab + this._focusTarget(nextTarget, event); + } + } + + /** @internal Focus a target and prevent the native event. */ + private _focusTarget( + target: DtChartFocusTarget | NativeFocusTarget | null, + event: FocusEvent | KeyboardEvent, + ): void { + if (target) { + if (event.preventDefault) { + event.preventDefault(); + } + target.focus(); + } + } + + /** @internal Find the next/prev target to focus. */ + private _findTargets(): { + nextTarget: DtChartFocusTarget | NativeFocusTarget | null; + prevTarget: DtChartFocusTarget | NativeFocusTarget | null; + } { + const element = this._elementRef.nativeElement as Element; + const nextTarget = this._findTarget( + this.nextTarget, + element.nextElementSibling, + ); + const prevTarget = this._findTarget( + this.prevTarget, + element.parentElement?.children[0], + (currentElement) => currentElement === element, + ); + return { nextTarget, prevTarget }; + } + + /** @internal Find a target to focus. */ + private _findTarget( + targetName?: string, + startingElement?: Element | null, + shouldStopFn?: (currentElement: Element) => boolean, + ): DtChartFocusTarget | NativeFocusTarget | null { + if (targetName) { + return this._findKnownTarget(targetName); + } else if (startingElement) { + return ( + findNextFocusableElement( + startingElement, + (element) => this._checker.isFocusable(element as HTMLElement), + shouldStopFn, + ) || null + ); + } + return null; + } + + /** @internal Find a known target to focus by name. */ + private _findKnownTarget(targetName: string): DtChartFocusTarget | null { + for (const target of this._chart._focusTargets) { + if (target.dtChartFocusTarget === targetName) { + return target; + } + } + return null; + } +} + +/** Directive to mare a focusable element by name so it can be found by the anchor. */ +@Directive({ + selector: '[dtChartFocusTarget]', +}) +export class DtChartFocusTarget implements OnDestroy { + @Input() dtChartFocusTarget: string; + + constructor(private _chart: DtChart, private _element: ElementRef) { + this._chart._focusTargets.add(this); + } + + ngOnDestroy(): void { + this._chart._focusTargets.delete(this); + } + + focus(): void { + this._element.nativeElement.focus(); + } +} + +/** Find the next focusable element after a provided starting element. */ +function findNextFocusableElement( + startingElement: Element, + isFocusable: (node: Element) => boolean, + shouldStopFn?: (nextElement: Element) => boolean, +): (Element & { focus: () => void }) | null { + let nextElement: Element | null = startingElement; + const shouldStop = shouldStopFn || (() => false); + while (nextElement && !shouldStop(nextElement)) { + if (isFocusable(nextElement)) { + return nextElement as (Element & { focus: () => void }) | null; + } + if (startingElement.children.length > 0) { + const focusableElement = findNextFocusableElement( + startingElement.children.item(0)!, + isFocusable, + shouldStop, + ); + if (focusableElement) { + return focusableElement; + } + } + nextElement = nextElement.nextElementSibling; + } + return null; +} diff --git a/libs/barista-components/chart/src/chart-module.ts b/libs/barista-components/chart/src/chart-module.ts index d01d73cbb1..64e61ee357 100644 --- a/libs/barista-components/chart/src/chart-module.ts +++ b/libs/barista-components/chart/src/chart-module.ts @@ -31,6 +31,7 @@ import { DtChartSelectionAreaAction } from './selection-area/overlay-action'; import { DtChartSelectionArea } from './selection-area/selection-area'; import { DtChartTimestamp } from './timestamp/timestamp'; import { DtChartTooltip } from './tooltip/chart-tooltip'; +import { DtChartFocusAnchor, DtChartFocusTarget } from './chart-focus-anchor'; /** components that should be declared and exported */ const COMPONENTS = [ @@ -40,6 +41,8 @@ const COMPONENTS = [ DtChartTimestamp, DtChartTooltip, DtChartSelectionAreaAction, + DtChartFocusAnchor, + DtChartFocusTarget, ]; @NgModule({ diff --git a/libs/barista-components/chart/src/chart.ts b/libs/barista-components/chart/src/chart.ts index c9f513d54f..12e65162e6 100644 --- a/libs/barista-components/chart/src/chart.ts +++ b/libs/barista-components/chart/src/chart.ts @@ -104,6 +104,7 @@ import { DtChartRange } from './range/range'; import { DtChartTimestamp } from './timestamp/timestamp'; import { DtChartTooltip } from './tooltip/chart-tooltip'; import { getPlotBackgroundInfo, retainSeriesVisibility } from './utils'; +import { DtChartFocusTarget } from './chart-focus-anchor'; const HIGHCHARTS_PLOT_BACKGROUND = '.highcharts-plot-background'; @@ -305,6 +306,8 @@ export class DtChart @ContentChild(DtChartTimestamp) _timestamp?: DtChartTimestamp; + _focusTargets = new Set(); + private _series?: Observable | DtChartSeries[]; private _currentSeries?: DtChartSeries[]; private _currentOptions: DtChartOptions; diff --git a/libs/barista-components/chart/src/range/range.html b/libs/barista-components/chart/src/range/range.html index 4a3e4b3a66..2d39412dfd 100644 --- a/libs/barista-components/chart/src/range/range.html +++ b/libs/barista-components/chart/src/range/range.html @@ -1,4 +1,9 @@ + {{ range | dtDateRange }} +
+ +
diff --git a/libs/barista-components/chart/src/range/range.spec.ts b/libs/barista-components/chart/src/range/range.spec.ts index 7d4460210c..38a1eaba5c 100644 --- a/libs/barista-components/chart/src/range/range.spec.ts +++ b/libs/barista-components/chart/src/range/range.spec.ts @@ -23,6 +23,7 @@ import { Component, DebugElement, OnInit, ViewChild } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; import { + DtChart, DtChartModule, DtChartRange, } from '@dynatrace/barista-components/chart'; @@ -31,6 +32,7 @@ import { dispatchFakeEvent, createKeyboardEvent, } from '@dynatrace/testing/browser'; +import { DtChartFocusTarget } from '../chart-focus-anchor'; import { ARIA_DEFAULT_LEFT_HANDLE_LABEL, ARIA_DEFAULT_RIGHT_HANDLE_LABEL, @@ -41,6 +43,10 @@ import { RangeStateChangedEvent } from './range'; // tslint:disable:no-magic-numbers no-unbound-method no-use-before-declare +export class MockChart { + _focusTargets = new Set(); +} + describe('DtChart Range', () => { beforeEach(() => { TestBed.configureTestingModule({ @@ -54,6 +60,7 @@ describe('DtChart Range', () => { RangeTestBindingValuesComponent, RangeA11yTestComponent, ], + providers: [{ provide: DtChart, useClass: MockChart }], }); TestBed.compileComponents(); }); diff --git a/libs/barista-components/chart/src/range/range.ts b/libs/barista-components/chart/src/range/range.ts index 9e2532d625..5a28fe0659 100644 --- a/libs/barista-components/chart/src/range/range.ts +++ b/libs/barista-components/chart/src/range/range.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { CdkTrapFocus } from '@angular/cdk/a11y'; import { coerceBooleanProperty, coerceNumberProperty, @@ -194,13 +193,6 @@ export class DtChartRange implements AfterViewInit, OnDestroy { @ViewChild(TemplateRef, { static: true }) _overlayTemplate: TemplateRef<{}>; - /** - * @internal The focus trap for the selected area, - * used by the selection area to chain the focus group of the area and the overlay. - */ - @ViewChild(CdkTrapFocus) - _selectedAreaFocusTrap: CdkTrapFocus; - /** * @internal * ElementRef of the selected area, used by the selection area to position diff --git a/libs/barista-components/chart/src/selection-area/selection-area.spec.ts b/libs/barista-components/chart/src/selection-area/selection-area.spec.ts index 31c98cf340..38c572b42f 100644 --- a/libs/barista-components/chart/src/selection-area/selection-area.spec.ts +++ b/libs/barista-components/chart/src/selection-area/selection-area.spec.ts @@ -236,22 +236,6 @@ describe('DtChart Selection Area', () => { ); expect(document.activeElement).toEqual(rangeContainer.nativeElement); }); - - it('should be in a focus trap after the chart range container was focused', () => { - range.focus(); - const rangeContainer = fixture.debugElement.query( - By.css('.dt-chart-range-container'), - ); - - const next = rangeContainer.nativeElement.nextSibling; - const prev = rangeContainer.nativeElement.previousSibling; - - expect(prev.classList.contains('cdk-focus-trap-anchor')).toBe(true); - expect(prev.getAttribute('tabindex')).toBe('0'); - - expect(next.classList.contains('cdk-focus-trap-anchor')).toBe(true); - expect(next.getAttribute('tabindex')).toBe('0'); - }); }); }); diff --git a/libs/barista-components/chart/src/selection-area/selection-area.ts b/libs/barista-components/chart/src/selection-area/selection-area.ts index 2264b0d352..59837e4c4a 100644 --- a/libs/barista-components/chart/src/selection-area/selection-area.ts +++ b/libs/barista-components/chart/src/selection-area/selection-area.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { FocusTrap, ConfigurableFocusTrapFactory } from '@angular/cdk/a11y'; import { ENTER } from '@angular/cdk/keycodes'; import { Overlay, @@ -84,7 +83,6 @@ import { } from '../timestamp/timestamp'; import { captureAndMergeEvents, - chainFocusTraps, getElementRef, getRelativeMousePosition, setPosition, @@ -160,9 +158,6 @@ export class DtChartSelectionArea /** The ref of the selection area overlay */ private _overlayRef: OverlayRef | null; - /** The focus trap inside the overlay */ - private _overlayFocusTrap: FocusTrap | null; - /** Template portal of the selection area overlay */ private _portal: TemplatePortal | null; @@ -184,7 +179,6 @@ export class DtChartSelectionArea constructor( @SkipSelf() private _chart: DtChart, private _elementRef: ElementRef, - private _focusTrapFactory: ConfigurableFocusTrapFactory, private _overlay: Overlay, private _zone: NgZone, private _viewportRuler: ViewportRuler, @@ -426,13 +420,6 @@ export class DtChartSelectionArea overlayRef.attach(this._portal); this._overlayRef = overlayRef; - - if (!this._overlayFocusTrap) { - this._overlayFocusTrap = this._focusTrapFactory.create( - this._overlayRef.overlayElement, - ); - this._attachFocusTrapListeners(); - } } /** Updates or creates an overlay for the range or timestamp. */ @@ -468,12 +455,6 @@ export class DtChartSelectionArea this._overlayRef.dispose(); } - // if we have a focus trap we have to destroy it - if (this._overlayFocusTrap) { - this._overlayFocusTrap.destroy(); - } - - this._overlayFocusTrap = null; this._overlayRef = null; this._portal = null; } @@ -941,33 +922,6 @@ export class DtChartSelectionArea }); } - /** Attaches the event listeners for the focus traps connected to each other */ - private _attachFocusTrapListeners(): void { - if (!this._overlayFocusTrap || !this._overlayRef) { - return; - } - - const traps = [this._overlayFocusTrap]; - const anchors = [this._overlayRef.hostElement]; - - if (this._chart._range && this._chart._range._rangeElementRef.first) { - traps.push(this._chart._range._selectedAreaFocusTrap.focusTrap); - anchors.push(this._chart._range._viewContainerRef.element.nativeElement); - } - - if ( - this._chart._timestamp && - this._chart._timestamp._timestampElementRef.first - ) { - traps.push(this._chart._timestamp._selectedFocusTrap.focusTrap); - anchors.push( - this._chart._timestamp._viewContainerRef.element.nativeElement, - ); - } - - chainFocusTraps(traps, anchors); - } - /** Filter function to check if the created range meets the maximum constraints */ private _isRangeInsideMaximumConstraint(range: { left: number; diff --git a/libs/barista-components/chart/src/timestamp/timestamp.html b/libs/barista-components/chart/src/timestamp/timestamp.html index 6c3c803e36..c2e1dd59ac 100644 --- a/libs/barista-components/chart/src/timestamp/timestamp.html +++ b/libs/barista-components/chart/src/timestamp/timestamp.html @@ -8,7 +8,6 @@ variant="secondary" (click)="_handleOverlayClose()" [attr.aria-label]="_ariaLabelClose" - cdkFocusInitial > @@ -17,7 +16,6 @@
; - /** - * @internal The focus trap for the selected timestamp, - * used by the selection area to chain the focus group of the timestamp and the overlay. - */ - @ViewChild(CdkTrapFocus) - _selectedFocusTrap: CdkTrapFocus; - /** * @internal * ElementRef of the timestamp, used by the selection area to position diff --git a/libs/barista-components/chart/src/utils.ts b/libs/barista-components/chart/src/utils.ts index d6b262a56e..840c3c73ce 100644 --- a/libs/barista-components/chart/src/utils.ts +++ b/libs/barista-components/chart/src/utils.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { FocusTrap } from '@angular/cdk/a11y'; import { DOWN_ARROW, LEFT_ARROW, @@ -214,58 +213,6 @@ export function getRelativeMousePosition( }; } -/** - * @internal - * Joins multiple focus traps together. - * @param traps The array of traps that should be joined - * @param trapContainers The trap containers of the traps in the same sort direction as the traps - */ -export function chainFocusTraps( - traps: FocusTrap[], - trapContainers: HTMLElement[], -): void { - if (traps.length !== trapContainers.length) { - return; - } - - // tslint:disable-next-line: one-variable-per-declaration - for (let i = 0, max = traps.length; i < max; i++) { - const anchors: HTMLElement[] = [].slice.call( - trapContainers[i].querySelectorAll('.cdk-focus-trap-anchor'), - ); - const nextTrap = i + 1 === max ? traps[0] : traps[i + 1]; - - // Focus trap has always two elements a start and an end - // tslint:disable-next-line: no-magic-numbers - if (anchors.length === 2) { - chainFocusToNextTrap(anchors[0], nextTrap, false); - chainFocusToNextTrap(anchors[1], nextTrap, true); - } - } -} - -/** - * @internal - * Chain Focus to another trap when a focus is triggered on a provided element - * @param target The target element where the focus should be captured - * @param nextTrap The next trap where the focus should be set - * @param first Wether the focus should be set to the last or first tabbable element. - */ -export function chainFocusToNextTrap( - target: HTMLElement, - nextTrap: FocusTrap, - first: boolean, -): void { - target.addEventListener('focus', (event: FocusEvent) => { - event.preventDefault(); - if (first) { - nextTrap.focusFirstTabbableElement(); - } else { - nextTrap.focusLastTabbableElement(); - } - }); -} - /** * @internal * Returns the plotBackground information (sizing and positioning) of a provided