Skip to content

Commit

Permalink
fix(datepicker): refine focus state checks
Browse files Browse the repository at this point in the history
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.

Closes #3494
  • Loading branch information
peterblazejewicz authored and gpolychronis-amadeus committed Jan 23, 2020
1 parent 5372506 commit 18e84c8
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 5 deletions.
2 changes: 2 additions & 0 deletions e2e-app/src/app/app.module.ts
Expand Up @@ -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';
Expand All @@ -35,6 +36,7 @@ import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-val
NavigationComponent,
DatepickerAutoCloseComponent,
DatepickerFocusComponent,
DatepickerMultipleComponent,
DropdownAutoCloseComponent,
DropdownFocusComponent,
DropdownPositionComponent,
Expand Down
4 changes: 3 additions & 1 deletion e2e-app/src/app/app.routing.ts
Expand Up @@ -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';
Expand All @@ -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}
]
},
{
Expand Down
6 changes: 3 additions & 3 deletions e2e-app/src/app/datepicker/datepicker.po.ts
@@ -1,4 +1,4 @@
import {$} from 'protractor';
import {$, $$} from 'protractor';

export class DatepickerPage {
getDatepicker(selector = 'ngb-datepicker') { return $(selector); }
Expand All @@ -7,9 +7,9 @@ export class DatepickerPage {

getToggle() { return $('#toggle'); }

getDayElement(date: Date) {
getDayElement(date: Date, index = 0) {
const ariaLabel = date.toLocaleString('en', {weekday: 'long', year: 'numeric', month: 'long', day: 'numeric'});
return this.getDatepicker().$(`div.ngb-dp-day[aria-label="${ariaLabel}"]`);
return $$('ngb-datepicker').get(index).$(`div.ngb-dp-day[aria-label="${ariaLabel}"]`);
}

getWeekdayElements() { return this.getDatepicker().$$('div.ngb-dp-weekday'); }
Expand Down
@@ -0,0 +1,14 @@
<h3>Datepicker focus tests</h3>

<form>
<div>
<ngb-datepicker name="dp1" placeholder="yyyy-mm-dd" #d1 [startDate]="startDate"></ngb-datepicker>
<ngb-datepicker name="dp2" placeholder="yyyy-mm-dd" #d2 [startDate]="startDate"></ngb-datepicker>

<span class="ml-3">startDate is
<span id="start-date">{{ startDate | json }}</span>
</span>

</div>

</form>
@@ -0,0 +1,6 @@
import {Component} from '@angular/core';

@Component({templateUrl: './datepicker-multiple.component.html'})
export class DatepickerMultipleComponent {
startDate = {year: 2016, month: 8};
}
@@ -0,0 +1,20 @@
import {Key} from 'protractor';
import {sendKey, openUrl} from '../../tools.po';
import {DatepickerMultiplePage} from './datepicker-multiple.po';

describe('Datepicker', () => {
let page: DatepickerMultiplePage;

beforeAll(() => page = new DatepickerMultiplePage());

beforeEach(async() => await openUrl('datepicker/multiple'));

it('should lose focus on tapping on a different datepicker instance', async() => {
await page.getDayElement(new Date(2016, 7, 1), 0).click();
await sendKey(Key.ARROW_DOWN);
await page.expectActive(new Date(2016, 7, 8), 0);
await page.getDayElement(new Date(2016, 7, 1), 1).click();
await sendKey(Key.ARROW_DOWN);
await page.expectActive(new Date(2016, 7, 8), 1);
});
});
8 changes: 8 additions & 0 deletions e2e-app/src/app/datepicker/multiple/datepicker-multiple.po.ts
@@ -0,0 +1,8 @@
import {DatepickerPage} from './../datepicker.po';

export class DatepickerMultiplePage extends DatepickerPage {
async expectActive(date: Date, index: number) {
const dateElement = this.getDayElement(date, index);
expect(await dateElement.$('.active').isPresent()).toBeTruthy(`The date should be active`);
}
}
2 changes: 2 additions & 0 deletions src/datepicker/datepicker-input.spec.ts
Expand Up @@ -364,6 +364,7 @@ describe('NgbInputDatepicker', () => {
<input ngbDatepicker [startDate]="{year: 2018, month: 3}" [(ngModel)]="date" #d="ngbDatepicker"
[ngModelOptions]="{updateOn: 'blur'}">`);

fixture.detectChanges();
const inputDebugEl = fixture.debugElement.query(By.css('input'));
const dpInput = fixture.debugElement.query(By.directive(NgbInputDatepicker)).injector.get(NgbInputDatepicker);

Expand Down Expand Up @@ -821,6 +822,7 @@ describe('NgbInputDatepicker', () => {

it('should add .ngb-dp-body class when attached to body', () => {
const fixture = createTestCmpt(`<input ngbDatepicker #d="ngbDatepicker" [container]="container">`);
fixture.detectChanges();
const dpInput = fixture.debugElement.query(By.directive(NgbInputDatepicker)).injector.get(NgbInputDatepicker);

// No container specified
Expand Down
4 changes: 3 additions & 1 deletion src/datepicker/datepicker.ts
Expand Up @@ -397,14 +397,16 @@ export class NgbDatepicker implements OnDestroy,
this._ngZone.runOutsideAngular(() => {
const focusIns$ = fromEvent<FocusEvent>(this._monthsEl.nativeElement, 'focusin');
const focusOuts$ = fromEvent<FocusEvent>(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
merge(focusIns$, focusOuts$)
.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'));
});
Expand Down

0 comments on commit 18e84c8

Please sign in to comment.