Skip to content

Commit

Permalink
fix(timepicker): fix tab navigation & handling keyboard arrows (#2053)
Browse files Browse the repository at this point in the history
FIxes #1836
  • Loading branch information
ymeine authored and maxokorokov committed May 13, 2019
1 parent 6c72421 commit 7efa35c
Show file tree
Hide file tree
Showing 9 changed files with 167 additions and 22 deletions.
21 changes: 17 additions & 4 deletions e2e-app/src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,28 @@ import {TooltipFocusComponent} from './tooltip/focus/tooltip-focus.component';
import {TooltipPositionComponent} from './tooltip/position/tooltip-position.component';
import {TypeaheadAutoCloseComponent} from './typeahead/autoclose/typeahead-autoclose.component';
import {TypeaheadFocusComponent} from './typeahead/focus/typeahead-focus.component';
import {TimepickerNavigationComponent} from './timepicker/navigation/timepicker-navigation.component';
import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-validation.component';


@NgModule({
declarations: [
AppComponent, NavigationComponent, DatepickerAutoCloseComponent, DatepickerFocusComponent,
DropdownAutoCloseComponent, DropdownFocusComponent, DropdownPositionComponent, ModalFocusComponent,
PopoverAutocloseComponent, TooltipAutocloseComponent, TooltipFocusComponent, TooltipPositionComponent,
TypeaheadFocusComponent, TypeaheadValidationComponent, TypeaheadAutoCloseComponent
AppComponent,
NavigationComponent,
DatepickerAutoCloseComponent,
DatepickerFocusComponent,
DropdownAutoCloseComponent,
DropdownFocusComponent,
DropdownPositionComponent,
ModalFocusComponent,
PopoverAutocloseComponent,
TooltipAutocloseComponent,
TooltipFocusComponent,
TooltipPositionComponent,
TypeaheadFocusComponent,
TypeaheadValidationComponent,
TypeaheadAutoCloseComponent,
TimepickerNavigationComponent,
],
imports: [BrowserModule, FormsModule, ReactiveFormsModule, routing, NgbModule],
bootstrap: [AppComponent]
Expand Down
15 changes: 12 additions & 3 deletions e2e-app/src/app/app.routing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {TooltipFocusComponent} from './tooltip/focus/tooltip-focus.component';
import {TooltipPositionComponent} from './tooltip/position/tooltip-position.component';
import {TypeaheadAutoCloseComponent} from './typeahead/autoclose/typeahead-autoclose.component';
import {TypeaheadFocusComponent} from './typeahead/focus/typeahead-focus.component';
import {TimepickerNavigationComponent} from './timepicker/navigation/timepicker-navigation.component';
import {TypeaheadValidationComponent} from './typeahead/validation/typeahead-validation.component';
import {DropdownPositionComponent} from './dropdown/position/dropdown-position.component';

Expand All @@ -24,14 +25,16 @@ export const routes: Routes = [
{path: 'autoclose', component: DatepickerAutoCloseComponent}
]
},
{path: 'modal', children: [{path: 'focus', component: ModalFocusComponent}]}, {
{path: 'modal', children: [{path: 'focus', component: ModalFocusComponent}]},
{
path: 'dropdown',
children: [
{path: 'autoclose', component: DropdownAutoCloseComponent}, {path: 'focus', component: DropdownFocusComponent},
{path: 'position', component: DropdownPositionComponent}
]
},
{path: 'popover', children: [{path: 'autoclose', component: PopoverAutocloseComponent}]}, {
{path: 'popover', children: [{path: 'autoclose', component: PopoverAutocloseComponent}]},
{
path: 'tooltip',
children: [
{path: 'autoclose', component: TooltipAutocloseComponent}, {path: 'focus', component: TooltipFocusComponent},
Expand All @@ -44,7 +47,13 @@ export const routes: Routes = [
{path: 'focus', component: TypeaheadFocusComponent}, {path: 'autoclose', component: TypeaheadAutoCloseComponent},
{path: 'validation', component: TypeaheadValidationComponent}
]
}
},
{
path: 'timepicker',
children: [
{path: 'navigation', component: TimepickerNavigationComponent},
]
},
];

export const routing: ModuleWithProviders = RouterModule.forRoot(routes, {useHash: true});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<input id="before" />

<ngb-timepicker
[seconds]="true"
[(ngModel)]="time"
></ngb-timepicker>

<input id="after" />
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import {Component} from '@angular/core';

@Component({
templateUrl: './timepicker-navigation.component.html',
})
export class TimepickerNavigationComponent {
time = {hour: 13, minute: 30, second: 30};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import {Key, ElementFinder} from 'protractor';

import {openUrl, expectFocused, sendKey, getCaretPosition} from '../../tools.po';

import {TimepickerNavigationPage} from './timepicker-navigation.po';

describe('Timepicker', () => {
let page: TimepickerNavigationPage;

beforeAll(() => page = new TimepickerNavigationPage());
beforeEach(async() => await openUrl('timepicker/navigation'));

async function expectCaretPosition(field: ElementFinder, position: number) {
const {start, end} = await getCaretPosition(field);
expect(end).toBe(end, 'Caret should be at single position (no range selected)');
expect(start).toBe(position, `Caret is not at proper position for given field`);
}

describe('navigation', () => {
it(`should jump between inputs`, async() => {
await page.focusInputBefore();

const[hourField, minuteField, secondField] = page.getFields();

await sendKey(Key.TAB);
await expectFocused(hourField, 'Hour field should be focused');
await sendKey(Key.TAB);
await expectFocused(minuteField, 'Minute field should be focused');
await sendKey(Key.TAB);
await expectFocused(secondField, 'Second field should be focused');
await sendKey(Key.TAB);
await expectFocused(page.getInputAfter(), 'Input after should be focused');

await sendKey(Key.SHIFT, Key.TAB);
await expectFocused(secondField, 'Second field should be focused');
await sendKey(Key.SHIFT, Key.TAB);
await expectFocused(minuteField, 'Minute field should be focused');
await sendKey(Key.SHIFT, Key.TAB);
await expectFocused(hourField, 'Hour field should be focused');
await sendKey(Key.SHIFT, Key.TAB);
await expectFocused(page.getInputBefore(), 'Input before should be focused');
});
});

describe('arrow keys', () => {
it(`should keep caret at the end of the input`, async() => {
const testField = async(fieldElement: ElementFinder) => {
await fieldElement.click();

const endPosition = 2;
const expectCaretAtEnd = () => expectCaretPosition(fieldElement, endPosition);

await sendKey(Key.END);
await expectCaretAtEnd();

await sendKey(Key.ARROW_UP);
await expectCaretAtEnd();

await sendKey(Key.ARROW_DOWN);
await expectCaretAtEnd();

await sendKey(Key.HOME);
await expectCaretPosition(fieldElement, 0);

await sendKey(Key.ARROW_UP);
await expectCaretAtEnd();

await sendKey(Key.ARROW_DOWN);
await expectCaretAtEnd();
};

for (const fieldElement of page.getFields()) {
await testField(fieldElement);
}
});
});
});
13 changes: 13 additions & 0 deletions e2e-app/src/app/timepicker/navigation/timepicker-navigation.po.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import {$} from 'protractor';

export type Field = 'hour' | 'minute' | 'second';

export class TimepickerNavigationPage {
getInputBefore() { return $('#before'); }
focusInputBefore() { return this.getInputBefore().click(); }

getInputAfter() { return $('#after'); }
getField(field: Field) { return $(`.ngb-tp-${field} > input`); }

getFields() { return ['hour', 'minute', 'second'].map((field: Field) => this.getField(field)); }
}
14 changes: 14 additions & 0 deletions e2e-app/src/app/tools.po.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,17 @@ export const openUrl = async(url: string) => {
await $(`#navigate-home`).click();
await $(`#navigate-${url.replace('/', '-')}`).click();
};

/**
* Returns the caret position ({start, end}) of the given element (must be an input).
*/
export async function getCaretPosition(element: ElementFinder) {
const[start, end] = await browser.executeScript<[number, number]>(
`
var element = arguments[0];
return [element.selectionStart, element.selectionEnd];
`,
element.getWebElement());

return {start, end};
}
12 changes: 6 additions & 6 deletions src/timepicker/timepicker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,12 +412,12 @@ describe('ngb-timepicker', () => {

const hourInput = getDebugInputs(fixture)[0];

hourInput.triggerEventHandler('keydown.ArrowUp', {}); // H+
hourInput.triggerEventHandler('keydown.ArrowUp', {preventDefault: () => {}}); // H+
fixture.detectChanges();
expectToDisplayTime(fixture.nativeElement, '11:30');
expect(fixture.componentInstance.model).toEqual({hour: 11, minute: 30, second: 0});

hourInput.triggerEventHandler('keydown.ArrowDown', {}); // H+
hourInput.triggerEventHandler('keydown.ArrowDown', {preventDefault: () => {}}); // H-
fixture.detectChanges();
expectToDisplayTime(fixture.nativeElement, '10:30');
expect(fixture.componentInstance.model).toEqual({hour: 10, minute: 30, second: 0});
Expand All @@ -441,12 +441,12 @@ describe('ngb-timepicker', () => {

const minuteInput = getDebugInputs(fixture)[1];

minuteInput.triggerEventHandler('keydown.ArrowUp', {}); // M+
minuteInput.triggerEventHandler('keydown.ArrowUp', {preventDefault: () => {}}); // M+
fixture.detectChanges();
expectToDisplayTime(fixture.nativeElement, '10:31');
expect(fixture.componentInstance.model).toEqual({hour: 10, minute: 31, second: 0});

minuteInput.triggerEventHandler('keydown.ArrowDown', {}); // M-
minuteInput.triggerEventHandler('keydown.ArrowDown', {preventDefault: () => {}}); // M-
fixture.detectChanges();
expectToDisplayTime(fixture.nativeElement, '10:30');
expect(fixture.componentInstance.model).toEqual({hour: 10, minute: 30, second: 0});
Expand All @@ -470,12 +470,12 @@ describe('ngb-timepicker', () => {

const secondInput = getDebugInputs(fixture)[2];

secondInput.triggerEventHandler('keydown.ArrowUp', {}); // S+
secondInput.triggerEventHandler('keydown.ArrowUp', {preventDefault: () => {}}); // S+
fixture.detectChanges();
expectToDisplayTime(fixture.nativeElement, '10:30:01');
expect(fixture.componentInstance.model).toEqual({hour: 10, minute: 30, second: 1});

secondInput.triggerEventHandler('keydown.ArrowDown', {}); // S-
secondInput.triggerEventHandler('keydown.ArrowDown', {preventDefault: () => {}}); // S-
fixture.detectChanges();
expectToDisplayTime(fixture.nativeElement, '10:30:00');
expect(fixture.componentInstance.model).toEqual({hour: 10, minute: 30, second: 0});
Expand Down
21 changes: 12 additions & 9 deletions src/timepicker/timepicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const NGB_TIMEPICKER_VALUE_ACCESSOR = {
<fieldset [disabled]="disabled" [class.disabled]="disabled">
<div class="ngb-tp">
<div class="ngb-tp-input-container ngb-tp-hour">
<button *ngIf="spinners" type="button" (click)="changeHour(hourStep)"
<button *ngIf="spinners" tabindex="-1" type="button" (click)="changeHour(hourStep)"
class="btn btn-link" [class.btn-sm]="isSmallSize" [class.btn-lg]="isLargeSize" [class.disabled]="disabled"
[disabled]="disabled">
<span class="chevron ngb-tp-chevron"></span>
Expand All @@ -41,8 +41,9 @@ const NGB_TIMEPICKER_VALUE_ACCESSOR = {
maxlength="2" placeholder="HH" i18n-placeholder="@@ngb.timepicker.HH"
[value]="formatHour(model?.hour)" (change)="updateHour($event.target.value)"
[readonly]="readonlyInputs" [disabled]="disabled" aria-label="Hours" i18n-aria-label="@@ngb.timepicker.hours"
(keydown.ArrowUp)="changeHour(hourStep)" (keydown.ArrowDown)="changeHour(-hourStep)">
<button *ngIf="spinners" type="button" (click)="changeHour(-hourStep)"
(keydown.ArrowUp)="changeHour(hourStep); $event.preventDefault()"
(keydown.ArrowDown)="changeHour(-hourStep); $event.preventDefault()">
<button *ngIf="spinners" tabindex="-1" type="button" (click)="changeHour(-hourStep)"
class="btn btn-link" [class.btn-sm]="isSmallSize" [class.btn-lg]="isLargeSize" [class.disabled]="disabled"
[disabled]="disabled">
<span class="chevron ngb-tp-chevron bottom"></span>
Expand All @@ -51,7 +52,7 @@ const NGB_TIMEPICKER_VALUE_ACCESSOR = {
</div>
<div class="ngb-tp-spacer">:</div>
<div class="ngb-tp-input-container ngb-tp-minute">
<button *ngIf="spinners" type="button" (click)="changeMinute(minuteStep)"
<button *ngIf="spinners" tabindex="-1" type="button" (click)="changeMinute(minuteStep)"
class="btn btn-link" [class.btn-sm]="isSmallSize" [class.btn-lg]="isLargeSize" [class.disabled]="disabled"
[disabled]="disabled">
<span class="chevron ngb-tp-chevron"></span>
Expand All @@ -61,8 +62,9 @@ const NGB_TIMEPICKER_VALUE_ACCESSOR = {
maxlength="2" placeholder="MM" i18n-placeholder="@@ngb.timepicker.MM"
[value]="formatMinSec(model?.minute)" (change)="updateMinute($event.target.value)"
[readonly]="readonlyInputs" [disabled]="disabled" aria-label="Minutes" i18n-aria-label="@@ngb.timepicker.minutes"
(keydown.ArrowUp)="changeMinute(minuteStep)" (keydown.ArrowDown)="changeMinute(-minuteStep)">
<button *ngIf="spinners" type="button" (click)="changeMinute(-minuteStep)"
(keydown.ArrowUp)="changeMinute(minuteStep); $event.preventDefault()"
(keydown.ArrowDown)="changeMinute(-minuteStep); $event.preventDefault()">
<button *ngIf="spinners" tabindex="-1" type="button" (click)="changeMinute(-minuteStep)"
class="btn btn-link" [class.btn-sm]="isSmallSize" [class.btn-lg]="isLargeSize" [class.disabled]="disabled"
[disabled]="disabled">
<span class="chevron ngb-tp-chevron bottom"></span>
Expand All @@ -71,7 +73,7 @@ const NGB_TIMEPICKER_VALUE_ACCESSOR = {
</div>
<div *ngIf="seconds" class="ngb-tp-spacer">:</div>
<div *ngIf="seconds" class="ngb-tp-input-container ngb-tp-second">
<button *ngIf="spinners" type="button" (click)="changeSecond(secondStep)"
<button *ngIf="spinners" tabindex="-1" type="button" (click)="changeSecond(secondStep)"
class="btn btn-link" [class.btn-sm]="isSmallSize" [class.btn-lg]="isLargeSize" [class.disabled]="disabled"
[disabled]="disabled">
<span class="chevron ngb-tp-chevron"></span>
Expand All @@ -81,8 +83,9 @@ const NGB_TIMEPICKER_VALUE_ACCESSOR = {
maxlength="2" placeholder="SS" i18n-placeholder="@@ngb.timepicker.SS"
[value]="formatMinSec(model?.second)" (change)="updateSecond($event.target.value)"
[readonly]="readonlyInputs" [disabled]="disabled" aria-label="Seconds" i18n-aria-label="@@ngb.timepicker.seconds"
(keydown.ArrowUp)="changeSecond(secondStep)" (keydown.ArrowDown)="changeSecond(-secondStep)">
<button *ngIf="spinners" type="button" (click)="changeSecond(-secondStep)"
(keydown.ArrowUp)="changeSecond(secondStep); $event.preventDefault()"
(keydown.ArrowDown)="changeSecond(-secondStep); $event.preventDefault()">
<button *ngIf="spinners" tabindex="-1" type="button" (click)="changeSecond(-secondStep)"
class="btn btn-link" [class.btn-sm]="isSmallSize" [class.btn-lg]="isLargeSize" [class.disabled]="disabled"
[disabled]="disabled">
<span class="chevron ngb-tp-chevron bottom"></span>
Expand Down

0 comments on commit 7efa35c

Please sign in to comment.