Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable keyboard navigation in the dropdown #2424

Merged
merged 32 commits into from
Oct 14, 2022
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
fafd525
💡 Add dummy TODO comment
jkaltoft Aug 16, 2022
d905d64
Work In Progress
jkaltoft Aug 18, 2022
537962f
🚧 Keep focusedIndex variable updated
jkaltoft Aug 18, 2022
96eef96
🚧 Clean up a bit
jkaltoft Aug 20, 2022
1c5f70d
🚧 Control arrow up/down depending on open/closed
jkaltoft Aug 20, 2022
12865dd
🚧 Select item on Enter/Space keypress
jkaltoft Aug 20, 2022
a9a8885
🐛 Only select focused item on enter/space when open
jkaltoft Aug 22, 2022
ab2fe2a
On opening move focus to selected index (if any)
jkaltoft Aug 22, 2022
309ff96
✅ Extend test
jkaltoft Aug 22, 2022
ae64812
✅ Update tests to match new behavior
jkaltoft Aug 22, 2022
524e543
Update todo-comment
jkaltoft Aug 23, 2022
0f13ede
♿️ Make Home and End key press behave properly
jkaltoft Aug 24, 2022
4483179
♿️ Focus on last item if key press ArrowUp
jkaltoft Aug 24, 2022
9e57b2f
⚗️ Try to remove focus outline on iOS
jkaltoft Aug 25, 2022
f30ecfc
🚧 stash commit
jkaltoft Sep 13, 2022
18d8ceb
Refactor logic to focus items
mark-drastrup Sep 22, 2022
61e001f
Remove the focusItem method
mark-drastrup Sep 22, 2022
3681ec8
Clean up code for focusing items
mark-drastrup Sep 22, 2022
1ff9a90
Remove ToDo comments
mark-drastrup Sep 23, 2022
b14e005
Utilize scrollIntoView to simplify scrolling logic
mark-drastrup Sep 23, 2022
634e965
Update cookbook example for custom item template
mark-drastrup Sep 23, 2022
c4c4e99
Merge branch 'develop' into feature/2342-dropdown-keyboard-navigation
mark-drastrup Sep 23, 2022
b682504
Remove unused CSS
mark-drastrup Oct 4, 2022
c25459b
Remove beforeEach and move the content into the it block, if there is…
mark-drastrup Oct 4, 2022
15aceec
Remove focused test
mark-drastrup Oct 4, 2022
0fc24d4
Merge branch 'develop' into feature/2342-dropdown-keyboard-navigation
mark-drastrup Oct 4, 2022
7b87251
Write test to check if focusedIndex is set when selecting an item
mark-drastrup Oct 6, 2022
eb87b73
Write tests for arrow key behaviour
mark-drastrup Oct 6, 2022
6e798d3
Merge branch 'develop' into feature/2342-dropdown-keyboard-navigation
mark-drastrup Oct 6, 2022
9b22d07
Merge branch 'develop' into feature/2342-dropdown-keyboard-navigation
mark-drastrup Oct 11, 2022
27328c3
Merge branch 'develop' into feature/2342-dropdown-keyboard-navigation
mark-drastrup Oct 12, 2022
8d31953
Remove todo comment
mark-drastrup Oct 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ const config = {
[items]="items"
itemTextProperty="title">
<kirby-item
*kirbyListItemTemplate="let item; let selected = selected"
*kirbyListItemTemplate="let item; let selected = selected; let focused = focused"
selectable="true"
[selected]="selected">
[selected]="selected"
[class.focused]="focused"
>
<kirby-icon *ngIf="selected" name="checkmark-selected" slot="start"></kirby-icon>
<kirby-label>
<h3>{{ item.title }}</h3>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,23 +309,39 @@ describe('DropdownComponent (popover version)', () => {
});

describe('and Space key is pressed', () => {
beforeEach(fakeAsync(() => {
it('should open dropdown', fakeAsync(() => {
spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'Space');
tick(openDelayInMs);
}));
it('should open dropdown', () => {

expect(spectator.component.isOpen).toBeTruthy();
});
}));
});

describe('and Enter key is pressed', () => {
beforeEach(fakeAsync(() => {
it('should open dropdown', fakeAsync(() => {
spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'Enter');
tick(openDelayInMs);

expect(spectator.component.isOpen).toBeTruthy();
}));
});

describe('and ArrowDown key is pressed', () => {
it('should open dropdown', fakeAsync(() => {
spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowDown');
tick(openDelayInMs);

expect(spectator.component.isOpen).toBeTruthy();
}));
it('should open dropdown', () => {
});

describe('and ArrowUp key is pressed', () => {
it('should open dropdown', fakeAsync(() => {
spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowUp');
tick(openDelayInMs);

expect(spectator.component.isOpen).toBeTruthy();
});
}));
});

describe('and first item is selected', () => {
Expand Down Expand Up @@ -378,144 +394,6 @@ describe('DropdownComponent (popover version)', () => {
});
});
});

const testMatrix = [
{
key: 'ArrowLeft',
scenario: [
{
selectedIndex: 1,
keypressCount: 1,
expectedIndex: 0,
},
{
selectedIndex: 2,
keypressCount: 1,
expectedIndex: 1,
},
{
selectedIndex: 2,
keypressCount: 2,
expectedIndex: 0,
},
],
},
{
key: 'ArrowUp',
scenario: [
{
selectedIndex: 1,
keypressCount: 1,
expectedIndex: 0,
},
{
selectedIndex: 2,
keypressCount: 1,
expectedIndex: 1,
},
{
selectedIndex: 2,
keypressCount: 2,
expectedIndex: 0,
},
],
},
{
key: 'ArrowRight',
scenario: [
{
selectedIndex: 0,
keypressCount: 1,
expectedIndex: 1,
},
{
selectedIndex: 0,
keypressCount: 2,
expectedIndex: 2,
},
{
selectedIndex: 1,
keypressCount: 1,
expectedIndex: 2,
},
],
},
{
key: 'ArrowDown',
scenario: [
{
selectedIndex: 0,
keypressCount: 1,
expectedIndex: 1,
},
{
selectedIndex: 0,
keypressCount: 2,
expectedIndex: 2,
},
{
selectedIndex: 1,
keypressCount: 1,
expectedIndex: 2,
},
],
},
{
key: 'Home',
scenario: [
{
selectedIndex: 1,
keypressCount: 1,
expectedIndex: 0,
},
{
selectedIndex: 2,
keypressCount: 1,
expectedIndex: 0,
},
{
selectedIndex: 4,
keypressCount: 1,
expectedIndex: 0,
},
],
},
{
key: 'End',
scenario: [
{
selectedIndex: 0,
keypressCount: 1,
expectedIndex: 4,
},
{
selectedIndex: 1,
keypressCount: 1,
expectedIndex: 4,
},
{
selectedIndex: 2,
keypressCount: 1,
expectedIndex: 4,
},
],
},
];

testMatrix.forEach((keyEvent) => {
keyEvent.scenario.forEach((scenario) => {
describe(`and selected item = ${scenario.selectedIndex} and ${keyEvent.key} key is pressed ${scenario.keypressCount} time(s)`, () => {
it(`should set selected item = ${scenario.expectedIndex}`, () => {
spectator.setInput('selectedIndex', scenario.selectedIndex);
for (let counter = 0; counter < scenario.keypressCount; counter++) {
spectator.dispatchKeyboardEvent(spectator.element, 'keydown', keyEvent.key);
}
expect(spectator.component.selectedIndex).toEqual(scenario.expectedIndex);
expect(spectator.component.value).toEqual(items[scenario.expectedIndex]);
});
});
});
});
});

describe('when open', () => {
Expand All @@ -540,9 +418,9 @@ describe('DropdownComponent (popover version)', () => {
});

describe('and Space key is pressed', () => {
it('should not close dropdown', () => {
it('should close dropdown', () => {
spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'Space');
expect(spectator.component.isOpen).toBeTruthy();
expect(spectator.component.isOpen).toBeFalsy();
});
});

Expand Down Expand Up @@ -700,6 +578,10 @@ describe('DropdownComponent (popover version)', () => {
for (let counter = 0; counter < scenario.keypressCount; counter++) {
spectator.dispatchKeyboardEvent(spectator.element, 'keydown', keyEvent.key);
}
// focused
expect(spectator.component.focusedIndex).toEqual(scenario.expectedIndex);
// selected
spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'Enter');
expect(spectator.component.selectedIndex).toEqual(scenario.expectedIndex);
expect(spectator.component.value).toEqual(items[scenario.expectedIndex]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,32 @@
<ng-container
*ngTemplateOutlet="
itemTemplate || defaultItemTemplate;
context: { $implicit: item, selected: i === selectedIndex, index: i }
context: {
$implicit: item,
selected: i === selectedIndex,
focused: i === focusedIndex,
index: i
}
"
></ng-container>
</ng-container>
</kirby-card>
</ng-template>

<ng-template #defaultItemTemplate let-item let-selected="selected" let-index="index">
<kirby-item [selectable]="true" [selected]="selected" (click)="onItemSelect(index)" role="option">
<ng-template
#defaultItemTemplate
let-item
let-selected="selected"
let-index="index"
let-focused="focused"
>
<kirby-item
[selectable]="true"
[selected]="selected"
(click)="onItemSelect(index)"
[class.focused]="focused"
role="option"
>
<!-- Tabindex fixes issue with popover dropdown not working in safari -->
<h3 tabindex="0">{{ getTextFromItem(item) }}</h3>
<kirby-icon *ngIf="selected" name="checkmark-selected" slot="end"></kirby-icon>
Expand Down
Loading