From fafd5258a0b509bba1b33bdb1c95676954c383d1 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Tue, 16 Aug 2022 16:43:24 +0200 Subject: [PATCH 01/27] =?UTF-8?q?=F0=9F=92=A1=20Add=20dummy=20TODO=20comme?= =?UTF-8?q?nt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/lib/components/dropdown/dropdown.component.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 0ee5a1c1f1..aa1a7245a5 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -516,6 +516,8 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue this.toggle(); } + // TODO: There will be changes - possibly here + @HostListener('keydown.arrowup', ['$event']) @HostListener('keydown.arrowdown', ['$event']) @HostListener('keydown.arrowleft', ['$event']) From d905d643767fc94d7ca9d6f2509862f967759bfe Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Thu, 18 Aug 2022 09:34:26 +0200 Subject: [PATCH 02/27] Work In Progress --- .../dropdown/dropdown.component.html | 1 + .../components/dropdown/dropdown.component.ts | 59 +++++++++++++++++-- .../lib/components/item/item.component.scss | 6 ++ 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.html b/libs/designsystem/src/lib/components/dropdown/dropdown.component.html index 7c081ca372..fd94aa7621 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.html +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.html @@ -39,6 +39,7 @@ +

{{ getTextFromItem(item) }}

diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index aa1a7245a5..9df9cb480e 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -71,6 +71,19 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue } } + // _focusedIndex keeps track of which element has focus and will be selected + // if it is activated (by pressing ENTER or SPACE key) + private _focusedIndex: number = -1; + get focusedIndex(): number { + return this._focusedIndex; + } + + @Input() set focusedIndex(value: number) { + if (this._focusedIndex !== value) { + this._focusedIndex = value; + } + } + @Input() itemTextProperty = 'text'; @@ -447,6 +460,36 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue } } + /** + * Give an Item focus by adding a "focused" CSS class + * @param index The Item that should get focus + */ + private focusItem(index: number) { + console.log(`focusItem(index = ${index})`); + const kirbyItems = + this.kirbyItemsSlotted && this.kirbyItemsSlotted.length + ? this.kirbyItemsSlotted + : this.kirbyItemsDefault; + if (kirbyItems && kirbyItems.length) { + // TODO: Remove focused class from all Items + kirbyItems.toArray().forEach((kirbyItem) => { + kirbyItem.nativeElement.classList.remove('focused'); + }); + + const selectedKirbyItem = kirbyItems.toArray()[index]; + + if (selectedKirbyItem && selectedKirbyItem.nativeElement) { + selectedKirbyItem.nativeElement.classList.add('focused'); + } + } + // if (index != this.selectedIndex) { + // this.selectedIndex = index; + // this.change.emit(this.value); + // this._onChange(this.value); + // this.scrollItemIntoView(index); + // } + } + @HostListener('keydown.tab', ['$event']) _onTab(event: KeyboardEvent) { if (this.isOpen) { @@ -495,11 +538,14 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @HostListener('blur', ['$event']) _onBlur(event?: FocusEvent) { - if (this.usePopover) return; - this.close(); - this._onTouched(); + // TODO: Enable this again + return; + // if (this.usePopover) return; + // this.close(); + // this._onTouched(); } + // TODO: This behavior may need to change @HostListener('keydown.space', ['$event']) _onSpace(event: KeyboardEvent) { event.preventDefault(); @@ -509,6 +555,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue } } + // TODO: This behavior may need to change @HostListener('keydown.enter', ['$event']) _onEnter(event: KeyboardEvent) { event.preventDefault(); @@ -528,9 +575,13 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue if (this.isOpen && (event.key === 'ArrowLeft' || event.key === 'ArrowRight')) { return; } + // TODO: Make service work for focused Items too const newIndex = this.keyboardHandlerService.handle(event, this.items, this.selectedIndex); if (newIndex > -1) { - this.selectItem(newIndex); + // TODO: Do not select item on arrow up/down - wait for ENTER/Space press + // this.selectItem(newIndex); + // TODO: Do apply focus and active styles + this.focusItem(newIndex); } return false; } diff --git a/libs/designsystem/src/lib/components/item/item.component.scss b/libs/designsystem/src/lib/components/item/item.component.scss index 6b37ae4451..7a74c3e315 100644 --- a/libs/designsystem/src/lib/components/item/item.component.scss +++ b/libs/designsystem/src/lib/components/item/item.component.scss @@ -8,6 +8,7 @@ position: relative; ion-item { + @include interaction-state.apply-focus-visible-background('xxxs'); @include interaction-state.apply-hover-ionic('xxxs'); @include interaction-state.apply-active-ionic('xxs'); @@ -92,6 +93,11 @@ --min-height: #{utils.$dropdown-item-height}; } +// Intented for use with keyboard navigation in +:host-context(kirby-dropdown .focused) ion-item { + --background: #{interaction-state.get-state-color('white', 'xxxs')}; +} + /* clean-css ignore:start */ // Fixes https://github.com/kirbydesign/designsystem/issues/1745 ion-item { From 537962fb407a116528106d0774036abeb7195010 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Thu, 18 Aug 2022 11:18:07 +0200 Subject: [PATCH 03/27] =?UTF-8?q?=F0=9F=9A=A7=20Keep=20focusedIndex=20vari?= =?UTF-8?q?able=20updated?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../components/dropdown/dropdown.component.ts | 24 ++++++++++++------- .../lib/components/item/item.component.scss | 2 ++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 9df9cb480e..47d9b92208 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -460,6 +460,12 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue } } + // TODO: We may not need this + onItemFocus(index: number) { + this.focusItem(index); + // this.close(); + } + /** * Give an Item focus by adding a "focused" CSS class * @param index The Item that should get focus @@ -482,12 +488,13 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue selectedKirbyItem.nativeElement.classList.add('focused'); } } - // if (index != this.selectedIndex) { - // this.selectedIndex = index; - // this.change.emit(this.value); - // this._onChange(this.value); - // this.scrollItemIntoView(index); - // } + + if (index !== this.focusedIndex) { + this.focusedIndex = index; + // this.change.emit(this.value); + // this._onChange(this.value); + this.scrollItemIntoView(index); + } } @HostListener('keydown.tab', ['$event']) @@ -575,11 +582,12 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue if (this.isOpen && (event.key === 'ArrowLeft' || event.key === 'ArrowRight')) { return; } - // TODO: Make service work for focused Items too + // TODO: Make service work for focused Items too/instead const newIndex = this.keyboardHandlerService.handle(event, this.items, this.selectedIndex); + // const newIndex = this.keyboardHandlerService.handle(event, this.items, this.focusedIndex); if (newIndex > -1) { // TODO: Do not select item on arrow up/down - wait for ENTER/Space press - // this.selectItem(newIndex); + this.selectItem(newIndex); // TODO: Do apply focus and active styles this.focusItem(newIndex); } diff --git a/libs/designsystem/src/lib/components/item/item.component.scss b/libs/designsystem/src/lib/components/item/item.component.scss index 7a74c3e315..6d894ac520 100644 --- a/libs/designsystem/src/lib/components/item/item.component.scss +++ b/libs/designsystem/src/lib/components/item/item.component.scss @@ -96,6 +96,8 @@ // Intented for use with keyboard navigation in :host-context(kirby-dropdown .focused) ion-item { --background: #{interaction-state.get-state-color('white', 'xxxs')}; + + // TODO: Try using Ionics custom properties (--background-focused etc) } /* clean-css ignore:start */ From 96eef965c24eecc7e968cc8d0aeb86c0402524f9 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Sat, 20 Aug 2022 12:50:13 +0200 Subject: [PATCH 04/27] =?UTF-8?q?=F0=9F=9A=A7=20Clean=20up=20a=20bit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../components/dropdown/dropdown.component.ts | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 47d9b92208..072e17a090 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -471,21 +471,21 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue * @param index The Item that should get focus */ private focusItem(index: number) { - console.log(`focusItem(index = ${index})`); const kirbyItems = this.kirbyItemsSlotted && this.kirbyItemsSlotted.length ? this.kirbyItemsSlotted : this.kirbyItemsDefault; + if (kirbyItems && kirbyItems.length) { - // TODO: Remove focused class from all Items + // "Un-focus" all Items kirbyItems.toArray().forEach((kirbyItem) => { kirbyItem.nativeElement.classList.remove('focused'); }); - const selectedKirbyItem = kirbyItems.toArray()[index]; + const focusedKirbyItem = kirbyItems.toArray()[index]; - if (selectedKirbyItem && selectedKirbyItem.nativeElement) { - selectedKirbyItem.nativeElement.classList.add('focused'); + if (focusedKirbyItem && focusedKirbyItem.nativeElement) { + focusedKirbyItem.nativeElement.classList.add('focused'); } } @@ -552,25 +552,24 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue // this._onTouched(); } - // TODO: This behavior may need to change + // TODO: Clean up + @HostListener('keydown.enter', ['$event']) @HostListener('keydown.space', ['$event']) _onSpace(event: KeyboardEvent) { - event.preventDefault(); - event.stopPropagation(); - if (!this.isOpen) { - this.open(); - } - } - - // TODO: This behavior may need to change - @HostListener('keydown.enter', ['$event']) - _onEnter(event: KeyboardEvent) { event.preventDefault(); event.stopPropagation(); this.toggle(); + // if (!this.isOpen) { + // this.open(); + // } } - // TODO: There will be changes - possibly here + // @HostListener('keydown.enter', ['$event']) + // _onEnter(event: KeyboardEvent) { + // event.preventDefault(); + // event.stopPropagation(); + // this.toggle(); + // } @HostListener('keydown.arrowup', ['$event']) @HostListener('keydown.arrowdown', ['$event']) From 1c5f70dcb820fde7c4d47f7a095327b55e78bbb8 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Sat, 20 Aug 2022 12:56:22 +0200 Subject: [PATCH 05/27] =?UTF-8?q?=F0=9F=9A=A7=20Control=20arrow=20up/down?= =?UTF-8?q?=20depending=20on=20open/closed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../lib/components/dropdown/dropdown.component.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 072e17a090..7337cba503 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -577,10 +577,23 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @HostListener('keydown.arrowright', ['$event']) _onArrowKeys(event: KeyboardEvent) { if (this.disabled) return; + // Mirror default HTML5 select behaviour - prevent left/right arrows when open: if (this.isOpen && (event.key === 'ArrowLeft' || event.key === 'ArrowRight')) { return; } + + // TODO: Use this if we want to mimic Material Dropdown Menu behavior + // if (!this.isOpen) return; + + // TODO: Use this if we want to mimic Carbon Dropdown behavior + if (!this.isOpen) { + // Avoid page scroll + event.preventDefault(); + this.open(); + return; + } + // TODO: Make service work for focused Items too/instead const newIndex = this.keyboardHandlerService.handle(event, this.items, this.selectedIndex); // const newIndex = this.keyboardHandlerService.handle(event, this.items, this.focusedIndex); From 12865ddf1ae7a0eaa5a56ba0eb17a1b7ce46d63e Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Sat, 20 Aug 2022 13:34:34 +0200 Subject: [PATCH 06/27] =?UTF-8?q?=F0=9F=9A=A7=20Select=20item=20on=20Enter?= =?UTF-8?q?/Space=20keypress?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../components/dropdown/dropdown.component.ts | 65 ++++++++++++++++--- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 7337cba503..a4cb6b8ab0 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -419,6 +419,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue private selectItem(index: number) { if (index != this.selectedIndex) { + this.focusItem(index); this.selectedIndex = index; this.change.emit(this.value); this._onChange(this.value); @@ -460,7 +461,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue } } - // TODO: We may not need this + // TODO: We probably do not need this onItemFocus(index: number) { this.focusItem(index); // this.close(); @@ -478,6 +479,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue if (kirbyItems && kirbyItems.length) { // "Un-focus" all Items + // TODO: Consider putting "Unfocusing" in its' own separate method kirbyItems.toArray().forEach((kirbyItem) => { kirbyItem.nativeElement.classList.remove('focused'); }); @@ -552,12 +554,29 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue // this._onTouched(); } - // TODO: Clean up + // TODO: Clean up - also rename private method + _onEnter() has been removed: is that a problem? @HostListener('keydown.enter', ['$event']) @HostListener('keydown.space', ['$event']) _onSpace(event: KeyboardEvent) { event.preventDefault(); event.stopPropagation(); + + // TODO: Move this block to a separate method + // TODO: keyboardHandlerService.handle() does not work for Enter/Space + // const newSelectedIndex = this.keyboardHandlerService.handle( + // event, + // this.items, + // this.selectedIndex + // ); + // console.log(`newSelectedIndex = ${newSelectedIndex}`); + // if (newSelectedIndex > -1) { + // // TODO: Do not select item on arrow up/down - wait for ENTER/Space press + // this.selectItem(newSelectedIndex); + // } + + // TODO: Idea: Could we just pass focusedIndex to selectItem()? + this.selectItem(this.focusedIndex); + this.toggle(); // if (!this.isOpen) { // this.open(); @@ -571,6 +590,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue // this.toggle(); // } + // TODO: Refactor/clean up _onArrowKeys() @HostListener('keydown.arrowup', ['$event']) @HostListener('keydown.arrowdown', ['$event']) @HostListener('keydown.arrowleft', ['$event']) @@ -594,18 +614,43 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue return; } - // TODO: Make service work for focused Items too/instead - const newIndex = this.keyboardHandlerService.handle(event, this.items, this.selectedIndex); - // const newIndex = this.keyboardHandlerService.handle(event, this.items, this.focusedIndex); - if (newIndex > -1) { - // TODO: Do not select item on arrow up/down - wait for ENTER/Space press - this.selectItem(newIndex); - // TODO: Do apply focus and active styles - this.focusItem(newIndex); + // TODO: Fork setting new index into separate indices for selected and focused + const newFocusedIndex = this.keyboardHandlerService.handle( + event, + this.items, + this.focusedIndex + ); + + if (newFocusedIndex > -1) { + this.focusItem(newFocusedIndex); } + + // // TODO: Move this to event listener for Enter and Space keypress + // const newSelectedIndex = this.keyboardHandlerService.handle( + // event, + // this.items, + // this.selectedIndex + // ); + // if (newSelectedIndex > -1) { + // // TODO: Do not select item on arrow up/down - wait for ENTER/Space press + // this.selectItem(newSelectedIndex); + // } + + // // TODO: Make service work for focused Items too/instead? + // const newIndex = this.keyboardHandlerService.handle(event, this.items, this.selectedIndex); + // // const newIndex = this.keyboardHandlerService.handle(event, this.items, this.focusedIndex); + // if (newIndex > -1) { + // // TODO: Do not select item on arrow up/down - wait for ENTER/Space press + // this.selectItem(newIndex); + // // TODO: Do apply focus and active styles + // this.focusItem(newIndex); + // } + + // TODO: Is this the best return value? Should other returns be changed to return booleans? return false; } + // TODO: Should HOME and END keys apply to focused? Both focused and selected? @HostListener('keydown.home', ['$event']) @HostListener('keydown.end', ['$event']) _onHomeEndKeys(event: KeyboardEvent) { From a9a888538b5177e8d7ce0c519e52ab877e589177 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Mon, 22 Aug 2022 13:39:27 +0200 Subject: [PATCH 07/27] =?UTF-8?q?=F0=9F=90=9B=20Only=20select=20focused=20?= =?UTF-8?q?item=20on=20enter/space=20when=20open?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/lib/components/dropdown/dropdown.component.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index a4cb6b8ab0..bbf02290e9 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -575,7 +575,9 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue // } // TODO: Idea: Could we just pass focusedIndex to selectItem()? - this.selectItem(this.focusedIndex); + if (this.isOpen) { + this.selectItem(this.focusedIndex); + } this.toggle(); // if (!this.isOpen) { From ab2fe2af1fdb2a76e545309f6c123935e895ac48 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Mon, 22 Aug 2022 13:46:04 +0200 Subject: [PATCH 08/27] On opening move focus to selected index (if any) --- .../src/lib/components/dropdown/dropdown.component.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index bbf02290e9..8e5955bdc1 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -343,6 +343,9 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue () => this.showDropdown(), DropdownComponent.OPEN_DELAY_IN_MS ); + + // Move focus to selected item (if any) + this.focusItem(this.selectedIndex); } } From 309ff96023978d28102e1f6187c1eb302e47b7d6 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Mon, 22 Aug 2022 15:13:50 +0200 Subject: [PATCH 09/27] =?UTF-8?q?=E2=9C=85=20Extend=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavior has been changed: ArrowUp/ArrowDown no longer selects Item. Instead they move focus to next item. Updated test checks that items get focused. Extended with a key press on Enter to trigger select Item. When selecting an item it also programmatically gets focused. --- .../components/dropdown/dropdown-popover.component.spec.ts | 4 ++++ .../src/lib/components/dropdown/dropdown.component.ts | 1 + 2 files changed, 5 insertions(+) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts b/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts index 298f659747..464a44f53a 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts @@ -700,6 +700,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); + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'Enter'); + // selected expect(spectator.component.selectedIndex).toEqual(scenario.expectedIndex); expect(spectator.component.value).toEqual(items[scenario.expectedIndex]); }); diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 8e5955bdc1..eb0ebb5b58 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -68,6 +68,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue if (this._selectedIndex != value) { this._selectedIndex = value; this._value = this.items[this.selectedIndex] || null; + this.focusItem(this._selectedIndex); } } From ae648123a82aaa21367860a9ea197e574029515f Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Mon, 22 Aug 2022 22:11:41 +0200 Subject: [PATCH 10/27] =?UTF-8?q?=E2=9C=85=20Update=20tests=20to=20match?= =?UTF-8?q?=20new=20behavior?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Key presses should not set selected item when dropdown is closed - ArrowDown and ArrowUp opens the dropdown - Space behaves like Enter (open and close dropdown) --- .../dropdown-popover.component.spec.ts | 164 +++--------------- .../dropdown/dropdown.component.spec.ts | 146 +--------------- .../components/dropdown/dropdown.component.ts | 8 +- 3 files changed, 33 insertions(+), 285 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts b/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts index 464a44f53a..28c2e0656b 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts @@ -328,6 +328,26 @@ describe('DropdownComponent (popover version)', () => { }); }); + describe('and ArrowDown key is pressed', () => { + beforeEach(fakeAsync(() => { + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowDown'); + tick(openDelayInMs); + })); + it('should open dropdown', () => { + expect(spectator.component.isOpen).toBeTruthy(); + }); + }); + + describe('and ArrowUp key is pressed', () => { + beforeEach(fakeAsync(() => { + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowUp'); + tick(openDelayInMs); + })); + it('should open dropdown', () => { + expect(spectator.component.isOpen).toBeTruthy(); + }); + }); + describe('and first item is selected', () => { beforeEach(() => { spectator.setInput('selectedIndex', 0); @@ -378,144 +398,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', () => { @@ -540,9 +422,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(); }); }); @@ -702,8 +584,8 @@ describe('DropdownComponent (popover version)', () => { } // focused expect(spectator.component.focusedIndex).toEqual(scenario.expectedIndex); - spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'Enter'); // selected + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'Enter'); expect(spectator.component.selectedIndex).toEqual(scenario.expectedIndex); expect(spectator.component.value).toEqual(items[scenario.expectedIndex]); }); diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts index 2e50753b3d..a5e31b3c3f 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts @@ -412,144 +412,6 @@ describe('DropdownComponent', () => { }); }); }); - - 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', () => { @@ -574,9 +436,9 @@ describe('DropdownComponent', () => { }); 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(); }); }); @@ -733,6 +595,10 @@ describe('DropdownComponent', () => { 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]); }); diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index eb0ebb5b58..1aaad62850 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -552,10 +552,10 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @HostListener('blur', ['$event']) _onBlur(event?: FocusEvent) { // TODO: Enable this again - return; - // if (this.usePopover) return; - // this.close(); - // this._onTouched(); + // return; + if (this.usePopover) return; + this.close(); + this._onTouched(); } // TODO: Clean up - also rename private method + _onEnter() has been removed: is that a problem? From 524e5437995e800c5f7086eb74fd4020e412ba8f Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Tue, 23 Aug 2022 10:25:19 +0200 Subject: [PATCH 11/27] Update todo-comment --- .../src/lib/components/dropdown/dropdown.component.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 1aaad62850..60e0f8b5f6 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -551,8 +551,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @HostListener('blur', ['$event']) _onBlur(event?: FocusEvent) { - // TODO: Enable this again - // return; + // TODO: Disable this for easier inspection in browser if (this.usePopover) return; this.close(); this._onTouched(); From 0f13ede9c9e9f62017019cbb487c0cf40420d944 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Wed, 24 Aug 2022 13:16:04 +0200 Subject: [PATCH 12/27] =?UTF-8?q?=E2=99=BF=EF=B8=8F=20Make=20Home=20and=20?= =?UTF-8?q?End=20key=20press=20behave=20properly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not select item - only focus. Similar to arrow keys. Should only apply when dropdown is open. --- .../lib/components/dropdown/dropdown.component.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 60e0f8b5f6..86f3f4bb00 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -660,9 +660,15 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @HostListener('keydown.end', ['$event']) _onHomeEndKeys(event: KeyboardEvent) { if (this.disabled) return; - const newIndex = this.keyboardHandlerService.handle(event, this.items, this.selectedIndex); - if (newIndex > -1) { - this.selectItem(newIndex); + if (!this.isOpen) return; + + const newFocusedIndex = this.keyboardHandlerService.handle( + event, + this.items, + this.focusedIndex + ); + if (newFocusedIndex > -1) { + this.focusItem(newFocusedIndex); } return false; } From 44831795ba64d6eb07b4646793385bab10b56af8 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Wed, 24 Aug 2022 15:24:26 +0200 Subject: [PATCH 13/27] =?UTF-8?q?=E2=99=BF=EF=B8=8F=20Focus=20on=20last=20?= =?UTF-8?q?item=20if=20key=20press=20ArrowUp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If no item is selected then move focus to last item if ArrowUp Move focus to first item if ArrowDown. --- .../lib/components/dropdown/dropdown.component.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 86f3f4bb00..70846c2638 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -616,6 +616,21 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue // Avoid page scroll event.preventDefault(); this.open(); + + // If no selected item then focus first or last item + if (this.selectedIndex < 0) { + switch (event.key) { + case 'ArrowUp': + this.focusItem(this.items.length - 1); + break; + case 'ArrowDown': + this.focusItem(0); + break; + default: + break; + } + } + return; } From 9e57b2f8eee301d1a735aaf8418836f16c87032a Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Thu, 25 Aug 2022 14:28:07 +0200 Subject: [PATCH 14/27] =?UTF-8?q?=E2=9A=97=EF=B8=8F=20Try=20to=20remove=20?= =?UTF-8?q?focus=20outline=20on=20iOS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/lib/components/dropdown/dropdown.component.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss b/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss index 669c9f653f..8cf4a19f7a 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss @@ -12,6 +12,11 @@ $min-screen-width: 375px; border-radius: utils.$border-radius-round; } + // TODO: Improve this - an attempt to remove focus outline on iOS + &:focus { + outline: none; + } + display: inline-block; position: relative; max-width: calc(100vw - #{$margin-horizontal-total}); From f30ecfc104c0bf03c495055e9d6e4ecab246f7a2 Mon Sep 17 00:00:00 2001 From: Jesper Kaltoft Vind Date: Tue, 13 Sep 2022 13:23:46 +0200 Subject: [PATCH 15/27] =?UTF-8?q?=F0=9F=9A=A7=20stash=20commit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/lib/components/dropdown/dropdown.component.scss | 8 ++++---- .../src/lib/components/dropdown/dropdown.component.ts | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss b/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss index 8cf4a19f7a..fe84da13b7 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss @@ -12,10 +12,10 @@ $min-screen-width: 375px; border-radius: utils.$border-radius-round; } - // TODO: Improve this - an attempt to remove focus outline on iOS - &:focus { - outline: none; - } + // // TODO: Improve this - an attempt to remove focus outline on iOS + // &:focus { + // outline: none; + // } display: inline-block; position: relative; diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 70846c2638..16bfe9bdf8 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -495,10 +495,13 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue } } + console.log(`index = ${index}, this.focusedIndex = ${this.focusedIndex}`); + if (index !== this.focusedIndex) { this.focusedIndex = index; // this.change.emit(this.value); // this._onChange(this.value); + console.log(`this.scrollItemIntoView(${index})`); this.scrollItemIntoView(index); } } From 18d8ceb2527c4422ee7cab4e03de0269e79f251a Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Thu, 22 Sep 2022 13:11:39 +0200 Subject: [PATCH 16/27] Refactor logic to focus items --- .../dropdown/dropdown.component.html | 23 ++++++++++++++++--- .../components/dropdown/dropdown.component.ts | 21 ----------------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.html b/libs/designsystem/src/lib/components/dropdown/dropdown.component.html index fd94aa7621..763f9d7c81 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.html +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.html @@ -31,16 +31,33 @@ - + - +

{{ getTextFromItem(item) }}

diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 16bfe9bdf8..e61594edf9 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -476,27 +476,6 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue * @param index The Item that should get focus */ private focusItem(index: number) { - const kirbyItems = - this.kirbyItemsSlotted && this.kirbyItemsSlotted.length - ? this.kirbyItemsSlotted - : this.kirbyItemsDefault; - - if (kirbyItems && kirbyItems.length) { - // "Un-focus" all Items - // TODO: Consider putting "Unfocusing" in its' own separate method - kirbyItems.toArray().forEach((kirbyItem) => { - kirbyItem.nativeElement.classList.remove('focused'); - }); - - const focusedKirbyItem = kirbyItems.toArray()[index]; - - if (focusedKirbyItem && focusedKirbyItem.nativeElement) { - focusedKirbyItem.nativeElement.classList.add('focused'); - } - } - - console.log(`index = ${index}, this.focusedIndex = ${this.focusedIndex}`); - if (index !== this.focusedIndex) { this.focusedIndex = index; // this.change.emit(this.value); From 61e001f96f0a92da49fa7b95e61b3e949456c73a Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Thu, 22 Sep 2022 13:46:23 +0200 Subject: [PATCH 17/27] Remove the focusItem method --- .../components/dropdown/dropdown.component.ts | 41 ++++--------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index e61594edf9..7c7a218dd7 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -68,7 +68,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue if (this._selectedIndex != value) { this._selectedIndex = value; this._value = this.items[this.selectedIndex] || null; - this.focusItem(this._selectedIndex); + this.scrollItemIntoView(this._selectedIndex); } } @@ -82,6 +82,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @Input() set focusedIndex(value: number) { if (this._focusedIndex !== value) { this._focusedIndex = value; + this.scrollItemIntoView(this._focusedIndex); } } @@ -345,8 +346,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue DropdownComponent.OPEN_DELAY_IN_MS ); - // Move focus to selected item (if any) - this.focusItem(this.selectedIndex); + this.scrollItemIntoView(this.selectedIndex); } } @@ -423,11 +423,10 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue private selectItem(index: number) { if (index != this.selectedIndex) { - this.focusItem(index); this.selectedIndex = index; + this.focusedIndex = index; this.change.emit(this.value); this._onChange(this.value); - this.scrollItemIntoView(index); } } @@ -465,26 +464,6 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue } } - // TODO: We probably do not need this - onItemFocus(index: number) { - this.focusItem(index); - // this.close(); - } - - /** - * Give an Item focus by adding a "focused" CSS class - * @param index The Item that should get focus - */ - private focusItem(index: number) { - if (index !== this.focusedIndex) { - this.focusedIndex = index; - // this.change.emit(this.value); - // this._onChange(this.value); - console.log(`this.scrollItemIntoView(${index})`); - this.scrollItemIntoView(index); - } - } - @HostListener('keydown.tab', ['$event']) _onTab(event: KeyboardEvent) { if (this.isOpen) { @@ -590,10 +569,6 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue return; } - // TODO: Use this if we want to mimic Material Dropdown Menu behavior - // if (!this.isOpen) return; - - // TODO: Use this if we want to mimic Carbon Dropdown behavior if (!this.isOpen) { // Avoid page scroll event.preventDefault(); @@ -603,10 +578,10 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue if (this.selectedIndex < 0) { switch (event.key) { case 'ArrowUp': - this.focusItem(this.items.length - 1); + this.focusedIndex = this.items.length - 1; break; case 'ArrowDown': - this.focusItem(0); + this.focusedIndex = 0; break; default: break; @@ -624,7 +599,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue ); if (newFocusedIndex > -1) { - this.focusItem(newFocusedIndex); + this.focusedIndex = newFocusedIndex; } // // TODO: Move this to event listener for Enter and Space keypress @@ -665,7 +640,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue this.focusedIndex ); if (newFocusedIndex > -1) { - this.focusItem(newFocusedIndex); + this.focusedIndex = newFocusedIndex; } return false; } From 3681ec821a3a0fd135e27fe4b57f0bfadd564bce Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Thu, 22 Sep 2022 14:18:57 +0200 Subject: [PATCH 18/27] Clean up code for focusing items --- .../lib/components/dropdown/dropdown.component.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 7c7a218dd7..b1139a82a3 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -67,8 +67,8 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @Input() set selectedIndex(value: number) { if (this._selectedIndex != value) { this._selectedIndex = value; + this.focusedIndex = this._selectedIndex; this._value = this.items[this.selectedIndex] || null; - this.scrollItemIntoView(this._selectedIndex); } } @@ -346,7 +346,8 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue DropdownComponent.OPEN_DELAY_IN_MS ); - this.scrollItemIntoView(this.selectedIndex); + // Move focus to selected item (if any) + this.focusedIndex = this.selectedIndex; } } @@ -549,14 +550,6 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue // } } - // @HostListener('keydown.enter', ['$event']) - // _onEnter(event: KeyboardEvent) { - // event.preventDefault(); - // event.stopPropagation(); - // this.toggle(); - // } - - // TODO: Refactor/clean up _onArrowKeys() @HostListener('keydown.arrowup', ['$event']) @HostListener('keydown.arrowdown', ['$event']) @HostListener('keydown.arrowleft', ['$event']) @@ -591,7 +584,6 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue return; } - // TODO: Fork setting new index into separate indices for selected and focused const newFocusedIndex = this.keyboardHandlerService.handle( event, this.items, From 1ff9a9026149b2e160a14cfd1882f18050b9c357 Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Fri, 23 Sep 2022 13:24:56 +0200 Subject: [PATCH 19/27] Remove ToDo comments --- .../dropdown/dropdown.component.html | 1 - .../components/dropdown/dropdown.component.ts | 50 ++----------------- 2 files changed, 4 insertions(+), 47 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.html b/libs/designsystem/src/lib/components/dropdown/dropdown.component.html index 763f9d7c81..663435292a 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.html +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.html @@ -50,7 +50,6 @@ let-index="index" let-focused="focused" > - -1) { - // // TODO: Do not select item on arrow up/down - wait for ENTER/Space press - // this.selectItem(newSelectedIndex); - // } - - // TODO: Idea: Could we just pass focusedIndex to selectItem()? if (this.isOpen) { this.selectItem(this.focusedIndex); } this.toggle(); - // if (!this.isOpen) { - // this.open(); - // } } @HostListener('keydown.arrowup', ['$event']) @@ -555,11 +536,11 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @HostListener('keydown.arrowleft', ['$event']) @HostListener('keydown.arrowright', ['$event']) _onArrowKeys(event: KeyboardEvent) { - if (this.disabled) return; + if (this.disabled) return false; // Mirror default HTML5 select behaviour - prevent left/right arrows when open: if (this.isOpen && (event.key === 'ArrowLeft' || event.key === 'ArrowRight')) { - return; + return false; } if (!this.isOpen) { @@ -581,7 +562,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue } } - return; + return false; } const newFocusedIndex = this.keyboardHandlerService.handle( @@ -594,32 +575,9 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue this.focusedIndex = newFocusedIndex; } - // // TODO: Move this to event listener for Enter and Space keypress - // const newSelectedIndex = this.keyboardHandlerService.handle( - // event, - // this.items, - // this.selectedIndex - // ); - // if (newSelectedIndex > -1) { - // // TODO: Do not select item on arrow up/down - wait for ENTER/Space press - // this.selectItem(newSelectedIndex); - // } - - // // TODO: Make service work for focused Items too/instead? - // const newIndex = this.keyboardHandlerService.handle(event, this.items, this.selectedIndex); - // // const newIndex = this.keyboardHandlerService.handle(event, this.items, this.focusedIndex); - // if (newIndex > -1) { - // // TODO: Do not select item on arrow up/down - wait for ENTER/Space press - // this.selectItem(newIndex); - // // TODO: Do apply focus and active styles - // this.focusItem(newIndex); - // } - - // TODO: Is this the best return value? Should other returns be changed to return booleans? return false; } - // TODO: Should HOME and END keys apply to focused? Both focused and selected? @HostListener('keydown.home', ['$event']) @HostListener('keydown.end', ['$event']) _onHomeEndKeys(event: KeyboardEvent) { From b14e0055e77ddec0570df2d6d07c76f54fa184ad Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Fri, 23 Sep 2022 13:28:33 +0200 Subject: [PATCH 20/27] Utilize scrollIntoView to simplify scrolling logic --- .../src/lib/components/dropdown/dropdown.component.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 735a3e0daf..3789371fee 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -451,16 +451,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue const selectedKirbyItem = kirbyItems.toArray()[index]; if (selectedKirbyItem && selectedKirbyItem.nativeElement) { const itemElement = selectedKirbyItem.nativeElement; - const scrollContainer = this.cardElement.nativeElement; - const itemTop = itemElement.offsetTop; - const itemBottom = itemElement.offsetTop + itemElement.offsetHeight; - const containerVisibleTop = scrollContainer.scrollTop; - const containerVisibleBottom = scrollContainer.clientHeight + scrollContainer.scrollTop; - if (itemTop < containerVisibleTop) { - scrollContainer.scrollTop = itemTop; - } else if (itemBottom > containerVisibleBottom) { - scrollContainer.scrollTop = itemBottom - scrollContainer.clientHeight; - } + itemElement.scrollIntoView({ block: 'nearest' }); } } } From 634e965c4477325e456506eb67dd13db91d4d906 Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Fri, 23 Sep 2022 14:08:37 +0200 Subject: [PATCH 21/27] Update cookbook example for custom item template --- .../dropdown-example/examples/custom-item-template.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/cookbook/src/app/examples/dropdown-example/examples/custom-item-template.ts b/apps/cookbook/src/app/examples/dropdown-example/examples/custom-item-template.ts index 6f64662566..cdbbc68157 100644 --- a/apps/cookbook/src/app/examples/dropdown-example/examples/custom-item-template.ts +++ b/apps/cookbook/src/app/examples/dropdown-example/examples/custom-item-template.ts @@ -8,9 +8,11 @@ const config = { [items]="items" itemTextProperty="title"> + [selected]="selected" + [class.focused]="focused" + >

{{ item.title }}

From b682504890ccb41b3f20c9079e23d734948eea43 Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Tue, 4 Oct 2022 09:32:05 +0200 Subject: [PATCH 22/27] Remove unused CSS --- .../src/lib/components/dropdown/dropdown.component.scss | 5 ----- 1 file changed, 5 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss b/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss index fe84da13b7..669c9f653f 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss @@ -12,11 +12,6 @@ $min-screen-width: 375px; border-radius: utils.$border-radius-round; } - // // TODO: Improve this - an attempt to remove focus outline on iOS - // &:focus { - // outline: none; - // } - display: inline-block; position: relative; max-width: calc(100vw - #{$margin-horizontal-total}); From c25459bf18ef357dfbe990ab07edc38756301d2b Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Tue, 4 Oct 2022 09:49:24 +0200 Subject: [PATCH 23/27] Remove beforeEach and move the content into the it block, if there is only 1 it block --- .../dropdown-popover.component.spec.ts | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts b/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts index 28c2e0656b..97e1d5e3d8 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts @@ -18,7 +18,7 @@ import { HorizontalDirection, PopoverComponent } from '../popover/popover.compon import { DropdownComponent } from './dropdown.component'; import { OpenState } from './dropdown.types'; -describe('DropdownComponent (popover version)', () => { +fdescribe('DropdownComponent (popover version)', () => { const items = [ { text: 'Item 1', value: 1 }, { text: 'Item 2', value: 2 }, @@ -309,43 +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); - })); - it('should open dropdown', () => { + expect(spectator.component.isOpen).toBeTruthy(); - }); + })); }); describe('and ArrowDown key is pressed', () => { - beforeEach(fakeAsync(() => { + it('should open dropdown', fakeAsync(() => { spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowDown'); tick(openDelayInMs); - })); - it('should open dropdown', () => { + expect(spectator.component.isOpen).toBeTruthy(); - }); + })); }); describe('and ArrowUp key is pressed', () => { - beforeEach(fakeAsync(() => { + it('should open dropdown', fakeAsync(() => { spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowUp'); tick(openDelayInMs); - })); - it('should open dropdown', () => { + expect(spectator.component.isOpen).toBeTruthy(); - }); + })); }); describe('and first item is selected', () => { From 15aceec0825582b2a62b603ca10adf27e67fc3cc Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Tue, 4 Oct 2022 09:49:44 +0200 Subject: [PATCH 24/27] Remove focused test --- .../lib/components/dropdown/dropdown-popover.component.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts b/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts index 97e1d5e3d8..bee5aa4970 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown-popover.component.spec.ts @@ -18,7 +18,7 @@ import { HorizontalDirection, PopoverComponent } from '../popover/popover.compon import { DropdownComponent } from './dropdown.component'; import { OpenState } from './dropdown.types'; -fdescribe('DropdownComponent (popover version)', () => { +describe('DropdownComponent (popover version)', () => { const items = [ { text: 'Item 1', value: 1 }, { text: 'Item 2', value: 2 }, From 7b872516aa512b6ea06b3b87c0475df7b017268a Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Thu, 6 Oct 2022 15:31:12 +0200 Subject: [PATCH 25/27] Write test to check if focusedIndex is set when selecting an item --- .../src/lib/components/dropdown/dropdown.component.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts index a5e31b3c3f..a91d67749c 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts @@ -20,7 +20,7 @@ import { OpenState } from './dropdown.types'; }) class OnPushHostComponent {} -describe('DropdownComponent', () => { +fdescribe('DropdownComponent', () => { const items = [ { text: 'Item 1', value: 1 }, { text: 'Item 2', value: 2 }, @@ -175,6 +175,10 @@ describe('DropdownComponent', () => { expect(spectator.component.selectedIndex).toEqual(newSelectedIndex); }); + fit('should have correct new focused index', () => { + expect(spectator.component.focusedIndex).toEqual(newSelectedIndex); + }); + it('should have correct new selected item', () => { expect(spectator.component.value).toEqual(expectedItem); }); From eb87b73adf71a4c84292c7794e6f9a65212cf98d Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Thu, 6 Oct 2022 21:31:33 +0200 Subject: [PATCH 26/27] Write tests for arrow key behaviour --- .../dropdown/dropdown.component.spec.ts | 68 ++++++++++++++++++- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts index a91d67749c..033b89f78f 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.spec.ts @@ -20,7 +20,7 @@ import { OpenState } from './dropdown.types'; }) class OnPushHostComponent {} -fdescribe('DropdownComponent', () => { +describe('DropdownComponent', () => { const items = [ { text: 'Item 1', value: 1 }, { text: 'Item 2', value: 2 }, @@ -175,7 +175,7 @@ fdescribe('DropdownComponent', () => { expect(spectator.component.selectedIndex).toEqual(newSelectedIndex); }); - fit('should have correct new focused index', () => { + it('should have correct new focused index', () => { expect(spectator.component.focusedIndex).toEqual(newSelectedIndex); }); @@ -382,6 +382,11 @@ fdescribe('DropdownComponent', () => { spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowUp'); expect(spectator.component.selectedIndex).toEqual(0); }); + + it('should not change focused item', () => { + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowUp'); + expect(spectator.component.focusedIndex).toEqual(0); + }); }); describe('and Home key is pressed', () => { it('should not change selected item', () => { @@ -408,6 +413,11 @@ fdescribe('DropdownComponent', () => { spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowDown'); expect(spectator.component.selectedIndex).toEqual(lastIndex); }); + + it('should not change focused item', () => { + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowDown'); + expect(spectator.component.focusedIndex).toEqual(lastIndex); + }); }); describe('and End key is pressed', () => { it('should not change selected item', () => { @@ -415,6 +425,58 @@ fdescribe('DropdownComponent', () => { expect(spectator.component.selectedIndex).toEqual(lastIndex); }); }); + + describe('and focused', () => { + beforeEach(() => { + spectator.element.focus(); + }); + + it('should open the dropdown when ArrowUp key is pressed', fakeAsync(() => { + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowUp'); + tick(openDelayInMs); + + expect(spectator.component.isOpen).toBeTruthy(); + })); + + it('should open the dropdown when ArrowDown key is pressed', fakeAsync(() => { + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowDown'); + tick(openDelayInMs); + + expect(spectator.component.isOpen).toBeTruthy(); + })); + + it('should highlight the first item in the list, if no item is selected and ArrowDown key is pressed', () => { + spectator.setInput('selectedIndex', -1); + + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowDown'); + + expect(spectator.component.focusedIndex).toEqual(0); + }); + + it('should highlight the last item in the list, if no item is selected and ArrowUp key is pressed', () => { + spectator.setInput('selectedIndex', -1); + + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowUp'); + + expect(spectator.component.focusedIndex).toEqual(4); + }); + + it('should highlight the selected item, when the ArrowUp key is pressed', () => { + spectator.setInput('selectedIndex', 2); + + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowUp'); + + expect(spectator.component.focusedIndex).toEqual(2); + }); + + it('should highlight the selected item, when the ArrowDown key is pressed', () => { + spectator.setInput('selectedIndex', 3); + + spectator.dispatchKeyboardEvent(spectator.element, 'keydown', 'ArrowDown'); + + expect(spectator.component.focusedIndex).toEqual(3); + }); + }); }); }); @@ -593,7 +655,7 @@ fdescribe('DropdownComponent', () => { testMatrix.forEach((keyEvent) => { keyEvent.scenario.forEach((scenario) => { - describe(`and selected item = ${scenario.selectedIndex} and ${keyEvent.key} key is pressed ${scenario.keypressCount} time(s)`, () => { + describe(`and selected item = ${scenario.selectedIndex} and focused 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++) { From 8d3195351bb598a65326bd5350e8ddf506703509 Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Wed, 12 Oct 2022 15:37:35 +0200 Subject: [PATCH 27/27] Remove todo comment --- libs/designsystem/src/lib/components/item/item.component.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/libs/designsystem/src/lib/components/item/item.component.scss b/libs/designsystem/src/lib/components/item/item.component.scss index 6d894ac520..7a74c3e315 100644 --- a/libs/designsystem/src/lib/components/item/item.component.scss +++ b/libs/designsystem/src/lib/components/item/item.component.scss @@ -96,8 +96,6 @@ // Intented for use with keyboard navigation in :host-context(kirby-dropdown .focused) ion-item { --background: #{interaction-state.get-state-color('white', 'xxxs')}; - - // TODO: Try using Ionics custom properties (--background-focused etc) } /* clean-css ignore:start */