From 035d3991f307184500725ad7be19e4ff91f93385 Mon Sep 17 00:00:00 2001 From: gpolychronisAmadeus <54309994+gpolychronisAmadeus@users.noreply.github.com> Date: Tue, 28 Jan 2020 15:00:29 +0100 Subject: [PATCH] fix(datepicker): refine focus state checks (#3549) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds additional condition to restrict focus in/out to instance of the datepicker by verifyng if focus event target and related targets are descendants of the datepicker element. This removes false results based only on the class name checks, which fail when focus is switched between datepicker instances. Fixes #3494 Co-authored-by: Piotr Błażejewicz (Peter Blazejewicz) --- e2e-app/src/app/app.module.ts | 2 ++ e2e-app/src/app/app.routing.ts | 4 +++- e2e-app/src/app/datepicker/datepicker.po.ts | 4 ++-- .../datepicker-multiple.component.html | 14 ++++++++++++ .../multiple/datepicker-multiple.component.ts | 6 +++++ .../multiple/datepicker-multiple.e2e-spec.ts | 22 +++++++++++++++++++ .../multiple/datepicker-multiple.po.ts | 10 +++++++++ src/datepicker/datepicker-input.spec.ts | 2 ++ src/datepicker/datepicker.ts | 4 +++- 9 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 e2e-app/src/app/datepicker/multiple/datepicker-multiple.component.html create mode 100644 e2e-app/src/app/datepicker/multiple/datepicker-multiple.component.ts create mode 100644 e2e-app/src/app/datepicker/multiple/datepicker-multiple.e2e-spec.ts create mode 100644 e2e-app/src/app/datepicker/multiple/datepicker-multiple.po.ts diff --git a/e2e-app/src/app/app.module.ts b/e2e-app/src/app/app.module.ts index cf0393d92d..a6bf2b9d27 100644 --- a/e2e-app/src/app/app.module.ts +++ b/e2e-app/src/app/app.module.ts @@ -10,6 +10,7 @@ import {NavigationComponent} from './navigation.component'; import {DatepickerAutoCloseComponent} from './datepicker/autoclose/datepicker-autoclose.component'; import {DatepickerFocusComponent} from './datepicker/focus/datepicker-focus.component'; +import {DatepickerMultipleComponent} from './datepicker/multiple/datepicker-multiple.component'; import {DropdownAutoCloseComponent} from './dropdown/autoclose/dropdown-autoclose.component'; import {DropdownFocusComponent} from './dropdown/focus/dropdown-focus.component'; import {DropdownPositionComponent} from './dropdown/position/dropdown-position.component'; @@ -35,6 +36,7 @@ import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-val NavigationComponent, DatepickerAutoCloseComponent, DatepickerFocusComponent, + DatepickerMultipleComponent, DropdownAutoCloseComponent, DropdownFocusComponent, DropdownPositionComponent, diff --git a/e2e-app/src/app/app.routing.ts b/e2e-app/src/app/app.routing.ts index f5546eba64..6d867fc8a3 100644 --- a/e2e-app/src/app/app.routing.ts +++ b/e2e-app/src/app/app.routing.ts @@ -3,6 +3,7 @@ import {RouterModule, Routes} from '@angular/router'; import {DatepickerAutoCloseComponent} from './datepicker/autoclose/datepicker-autoclose.component'; import {DatepickerFocusComponent} from './datepicker/focus/datepicker-focus.component'; +import {DatepickerMultipleComponent} from './datepicker/multiple/datepicker-multiple.component'; import {DropdownAutoCloseComponent} from './dropdown/autoclose/dropdown-autoclose.component'; import {DropdownFocusComponent} from './dropdown/focus/dropdown-focus.component'; import {DropdownPositionComponent} from './dropdown/position/dropdown-position.component'; @@ -27,7 +28,8 @@ export const routes: Routes = [ path: 'datepicker', children: [ {path: 'focus', component: DatepickerFocusComponent}, - {path: 'autoclose', component: DatepickerAutoCloseComponent} + {path: 'autoclose', component: DatepickerAutoCloseComponent}, + {path: 'multiple', component: DatepickerMultipleComponent} ] }, { diff --git a/e2e-app/src/app/datepicker/datepicker.po.ts b/e2e-app/src/app/datepicker/datepicker.po.ts index e9d48b86f8..56d581a064 100644 --- a/e2e-app/src/app/datepicker/datepicker.po.ts +++ b/e2e-app/src/app/datepicker/datepicker.po.ts @@ -7,9 +7,9 @@ export class DatepickerPage { getToggle() { return $('#toggle'); } - getDayElement(date: Date) { + getDayElement(date: Date, datepicker = this.getDatepicker()) { const ariaLabel = date.toLocaleString('en', {weekday: 'long', year: 'numeric', month: 'long', day: 'numeric'}); - return this.getDatepicker().$(`div.ngb-dp-day[aria-label="${ariaLabel}"]`); + return datepicker.$(`div.ngb-dp-day[aria-label="${ariaLabel}"]`); } getWeekdayElements() { return this.getDatepicker().$$('div.ngb-dp-weekday'); } diff --git a/e2e-app/src/app/datepicker/multiple/datepicker-multiple.component.html b/e2e-app/src/app/datepicker/multiple/datepicker-multiple.component.html new file mode 100644 index 0000000000..1498e4d99e --- /dev/null +++ b/e2e-app/src/app/datepicker/multiple/datepicker-multiple.component.html @@ -0,0 +1,14 @@ +

Datepicker focus tests

+ +
+
+ + + + startDate is + {{ startDate | json }} + + +
+ +
diff --git a/e2e-app/src/app/datepicker/multiple/datepicker-multiple.component.ts b/e2e-app/src/app/datepicker/multiple/datepicker-multiple.component.ts new file mode 100644 index 0000000000..4d9a098d6c --- /dev/null +++ b/e2e-app/src/app/datepicker/multiple/datepicker-multiple.component.ts @@ -0,0 +1,6 @@ +import {Component} from '@angular/core'; + +@Component({templateUrl: './datepicker-multiple.component.html'}) +export class DatepickerMultipleComponent { + startDate = {year: 2016, month: 8}; +} diff --git a/e2e-app/src/app/datepicker/multiple/datepicker-multiple.e2e-spec.ts b/e2e-app/src/app/datepicker/multiple/datepicker-multiple.e2e-spec.ts new file mode 100644 index 0000000000..a9e66f400d --- /dev/null +++ b/e2e-app/src/app/datepicker/multiple/datepicker-multiple.e2e-spec.ts @@ -0,0 +1,22 @@ +import {Key} from 'protractor'; +import {sendKey, openUrl} from '../../tools.po'; +import {DatepickerMultiplePage} from './datepicker-multiple.po'; + +describe('Datepicker multiple instances', () => { + let page: DatepickerMultiplePage; + + beforeAll(() => page = new DatepickerMultiplePage()); + + beforeEach(async() => await openUrl('datepicker/multiple')); + + it('the instance tapped should gain focus', async() => { + const dp1 = await page.getDatepicker('#dp1'); + const dp2 = await page.getDatepicker('#dp2'); + await page.getDayElement(new Date(2016, 7, 1), dp1).click(); + await sendKey(Key.ARROW_DOWN); + await page.expectActive(new Date(2016, 7, 8), dp1); + await page.getDayElement(new Date(2016, 7, 1), dp2).click(); + await sendKey(Key.ARROW_DOWN); + await page.expectActive(new Date(2016, 7, 8), dp2); + }); +}); diff --git a/e2e-app/src/app/datepicker/multiple/datepicker-multiple.po.ts b/e2e-app/src/app/datepicker/multiple/datepicker-multiple.po.ts new file mode 100644 index 0000000000..c98febc6ec --- /dev/null +++ b/e2e-app/src/app/datepicker/multiple/datepicker-multiple.po.ts @@ -0,0 +1,10 @@ +import {DatepickerPage} from './../datepicker.po'; +import {expectFocused} from '../../tools.po'; + +export class DatepickerMultiplePage extends DatepickerPage { + async expectActive(date: Date, datepicker) { + const dateElement = this.getDayElement(date, datepicker); + expect(await dateElement.$('.active').isPresent()).toBeTruthy(`The date should be active`); + await expectFocused(dateElement, `active date should be focused`); + } +} diff --git a/src/datepicker/datepicker-input.spec.ts b/src/datepicker/datepicker-input.spec.ts index 385959d49f..fc82822f9f 100644 --- a/src/datepicker/datepicker-input.spec.ts +++ b/src/datepicker/datepicker-input.spec.ts @@ -364,6 +364,7 @@ describe('NgbInputDatepicker', () => { `); + fixture.detectChanges(); const inputDebugEl = fixture.debugElement.query(By.css('input')); const dpInput = fixture.debugElement.query(By.directive(NgbInputDatepicker)).injector.get(NgbInputDatepicker); @@ -877,6 +878,7 @@ describe('NgbInputDatepicker', () => { it('should add .ngb-dp-body class when attached to body', () => { const fixture = createTestCmpt(``); + fixture.detectChanges(); const dpInput = fixture.debugElement.query(By.directive(NgbInputDatepicker)).injector.get(NgbInputDatepicker); // No container specified diff --git a/src/datepicker/datepicker.ts b/src/datepicker/datepicker.ts index c3336c7957..116cf9ccfd 100644 --- a/src/datepicker/datepicker.ts +++ b/src/datepicker/datepicker.ts @@ -397,6 +397,7 @@ export class NgbDatepicker implements OnDestroy, this._ngZone.runOutsideAngular(() => { const focusIns$ = fromEvent(this._monthsEl.nativeElement, 'focusin'); const focusOuts$ = fromEvent(this._monthsEl.nativeElement, 'focusout'); + const {nativeElement} = this._elementRef; // we're changing 'focusVisible' only when entering or leaving months view // and ignoring all focus events where both 'target' and 'related' target are day cells @@ -404,7 +405,8 @@ export class NgbDatepicker implements OnDestroy, .pipe( filter( ({target, relatedTarget}) => - !(hasClassName(target, 'ngb-dp-day') && hasClassName(relatedTarget, 'ngb-dp-day'))), + !(hasClassName(target, 'ngb-dp-day') && hasClassName(relatedTarget, 'ngb-dp-day') && + nativeElement.contains(target as Node) && nativeElement.contains(relatedTarget as Node))), takeUntil(this._destroyed$)) .subscribe(({type}) => this._ngZone.run(() => this._service.focusVisible = type === 'focusin')); });