From ac0e8a5f39252ee68bb954f586f37377b35cebed Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Wed, 12 Oct 2022 13:37:03 +0200 Subject: [PATCH 1/4] Toggle 'clicked' class to prevent focus ring on click --- .../dropdown/dropdown.component.scss | 4 ++++ .../components/dropdown/dropdown.component.ts | 20 +++++++++++++++++++ 2 files changed, 24 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..243df2c5b8 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.scss @@ -16,6 +16,10 @@ $min-screen-width: 375px; position: relative; max-width: calc(100vw - #{$margin-horizontal-total}); + &.clicked { + box-shadow: none; + } + &.expand { display: block; diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index 0ee5a1c1f1..5693af5575 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -218,6 +218,19 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue onToggle(event: MouseEvent) { event.stopPropagation(); + + /* The 'clicked' class is applied to prevent the dropdown from getting a focus ring on click. + There is a bug that causes the dropdown to get a focus ring on click, if it is the first element that is interacted with + after the page is loaded. If the user interacts with any other element before, then the dropdown won't get a focus ring. + See issue: https://github.com/kirbydesign/designsystem/issues/2477. + + This solution can potentially be refactored, when popover is not experimental anymore. Then it could be possible + to close the dropdown when the popover backdrop is clicked, instead of relying on the blur event, which is utilized + by this line below: this.elementRef.nativeElement.focus(). Right now this forces the blur event to be triggered, when + clicking outside of the modal. + */ + this.elementRef.nativeElement.classList.add('clicked'); + if (!this.isOpen) { this.elementRef.nativeElement.focus(); } @@ -453,6 +466,13 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue event.preventDefault(); this.close(); } + + if (this.elementRef.nativeElement.classList.contains('clicked')) { + // Remove the 'clicked' class if the user has previously opened the modal by clicking, + // since the class prevents the focus ring from showing, + // which is expected to happen, when using the tab key + this.elementRef.nativeElement.classList.remove('clicked'); + } } @HostListener('mousedown', ['$event']) From 65556e07a7ac5e613ceec85f63149d114e6ace01 Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Mon, 24 Oct 2022 18:43:06 +0900 Subject: [PATCH 2/4] Change comments to say 'dropdown' instead of 'modal' --- .../src/lib/components/dropdown/dropdown.component.ts | 4 ++-- 1 file changed, 2 insertions(+), 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 e6bc37d3c1..cce3d2c61c 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -242,7 +242,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue This solution can potentially be refactored, when popover is not experimental anymore. Then it could be possible to close the dropdown when the popover backdrop is clicked, instead of relying on the blur event, which is utilized by this line below: this.elementRef.nativeElement.focus(). Right now this forces the blur event to be triggered, when - clicking outside of the modal. + clicking outside of the dropdown. */ this.elementRef.nativeElement.classList.add('clicked'); @@ -477,7 +477,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue } if (this.elementRef.nativeElement.classList.contains('clicked')) { - // Remove the 'clicked' class if the user has previously opened the modal by clicking, + // Remove the 'clicked' class if the user has previously opened the dropdown by clicking, // since the class prevents the focus ring from showing, // which is expected to happen, when using the tab key this.elementRef.nativeElement.classList.remove('clicked'); From 64c653dd31934671cd33b271be90c27da06349ff Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Tue, 25 Oct 2022 19:52:50 +0900 Subject: [PATCH 3/4] Refactor to use Hostbinding to dynamically add 'clicked' class --- .../components/dropdown/dropdown.component.ts | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index cce3d2c61c..a3f5b88793 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -182,6 +182,19 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue return this.verticalDirection === VerticalDirection.up; } + /* The 'clicked' class is applied through Hostbinding to prevent the dropdown from getting a focus ring on click. + There is a bug that causes the dropdown to get a focus ring on click, if it is the first element that is interacted with + after the page is loaded. If the user interacts with any other element before, then the dropdown won't get a focus ring. + See issue: https://github.com/kirbydesign/designsystem/issues/2477. + + This solution can potentially be refactored, when popover is not experimental anymore. Then it could be possible + to close the dropdown when the popover backdrop is clicked, instead of relying on the blur event, which is utilized + by this line below: this.elementRef.nativeElement.focus(). Right now this forces the blur event to be triggered, when + clicking outside of the dropdown. + */ + @HostBinding('class.clicked') + clicked = false; + @ContentChild(ListItemTemplateDirective, { static: true, read: TemplateRef }) itemTemplate: TemplateRef; @ContentChildren(ListItemTemplateDirective, { read: ElementRef }) @@ -234,17 +247,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue onToggle(event: MouseEvent) { event.stopPropagation(); - /* The 'clicked' class is applied to prevent the dropdown from getting a focus ring on click. - There is a bug that causes the dropdown to get a focus ring on click, if it is the first element that is interacted with - after the page is loaded. If the user interacts with any other element before, then the dropdown won't get a focus ring. - See issue: https://github.com/kirbydesign/designsystem/issues/2477. - - This solution can potentially be refactored, when popover is not experimental anymore. Then it could be possible - to close the dropdown when the popover backdrop is clicked, instead of relying on the blur event, which is utilized - by this line below: this.elementRef.nativeElement.focus(). Right now this forces the blur event to be triggered, when - clicking outside of the dropdown. - */ - this.elementRef.nativeElement.classList.add('clicked'); + this.clicked = true; if (!this.isOpen) { this.elementRef.nativeElement.focus(); @@ -476,11 +479,11 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue this.close(); } - if (this.elementRef.nativeElement.classList.contains('clicked')) { - // Remove the 'clicked' class if the user has previously opened the dropdown by clicking, + if (this.clicked) { + // Remove the 'clicked' class (Hostbinding) if the user has previously opened the dropdown by clicking, // since the class prevents the focus ring from showing, // which is expected to happen, when using the tab key - this.elementRef.nativeElement.classList.remove('clicked'); + this.clicked = false; } } @@ -525,7 +528,6 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @HostListener('blur', ['$event']) _onBlur(event?: FocusEvent) { if (this.usePopover) return; - this.close(); this._onTouched(); } From 46f76336e47cf0038a6eff199bd0afa999550908 Mon Sep 17 00:00:00 2001 From: Mark Drastrup Date: Tue, 25 Oct 2022 19:57:43 +0900 Subject: [PATCH 4/4] Re-add method call, that was removed for test purposes --- .../src/lib/components/dropdown/dropdown.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts index a3f5b88793..bdb255a64f 100644 --- a/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts +++ b/libs/designsystem/src/lib/components/dropdown/dropdown.component.ts @@ -528,6 +528,7 @@ export class DropdownComponent implements AfterViewInit, OnDestroy, ControlValue @HostListener('blur', ['$event']) _onBlur(event?: FocusEvent) { if (this.usePopover) return; + this.close(); this._onTouched(); }