From ec31ceee1dc81fa6291036b658f762f4139a2e11 Mon Sep 17 00:00:00 2001 From: RivaIvanova Date: Thu, 15 May 2025 15:53:54 +0300 Subject: [PATCH 1/4] fix(tooltip): make touch events passive --- .../tooltip/tooltip-target.directive.ts | 14 +++++++--- .../tooltip/tooltip.directive.spec.ts | 16 ++++++++++- .../directives/tooltip/tooltip.directive.ts | 27 +++++++++++++++++-- .../lib/test-utils/tooltip-components.spec.ts | 4 ++- 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts index a26400879b6..502eabb2149 100644 --- a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts +++ b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts @@ -258,26 +258,28 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen /** * @hidden */ - @HostListener('touchstart') public onTouchStart() { if (this.tooltipDisabled) { return; } + this.target.tooltipTarget = this; this.showTooltip(); } /** * @hidden */ - @HostListener('document:touchstart', ['$event']) public onDocumentTouchStart(event) { if (this.tooltipDisabled) { return; } - if (this.nativeElement !== event.target && - !this.nativeElement.contains(event.target) + const tooltipTarget = this.target.tooltipTarget?.nativeElement; + + if (tooltipTarget && + tooltipTarget !== event.target && + !tooltipTarget.contains(event.target) ) { this.hideTooltip(); } @@ -308,6 +310,9 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen event.cancel = true; } }); + + this.nativeElement.addEventListener('touchstart', this.onTouchStart = this.onTouchStart.bind(this), { passive: true }); + this.target.onDocumentTouchStart = this.onDocumentTouchStart.bind(this); } /** @@ -315,6 +320,7 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen */ public ngOnDestroy() { this.hideTooltip(); + this.nativeElement.removeEventListener('touchstart', this.onTouchStart); this.destroy$.next(); this.destroy$.complete(); } diff --git a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts index 8ff5e020576..76ad8f8a537 100644 --- a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts +++ b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts @@ -276,7 +276,7 @@ describe('IgxTooltip', () => { flush(); verifyTooltipVisibility(tooltipNativeElement, tooltipTarget, true); - + fix.componentInstance.showButton = false; fix.detectChanges(); flush(); @@ -567,6 +567,20 @@ describe('IgxTooltip', () => { // Tooltip is NOT visible and positioned relative to buttonOne verifyTooltipPosition(tooltipNativeElement, buttonOne, false); })); + + it('Should not call `hideTooltip` multiple times on document:touchstart', fakeAsync(() => { + spyOn(targetOne, 'hideTooltip').and.callThrough(); + spyOn(targetTwo, 'hideTooltip').and.callThrough(); + + touchElement(buttonOne); + tick(500); + + const dummyDiv = fix.debugElement.query(By.css('.dummyDiv')); + touchElement(dummyDiv); + + expect(targetOne.hideTooltip).not.toHaveBeenCalled(); + expect(targetTwo.hideTooltip).toHaveBeenCalledTimes(1); + })); }); describe('Tooltip integration', () => { diff --git a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.ts b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.ts index 7cff12228af..e0947820207 100644 --- a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.ts +++ b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.ts @@ -1,10 +1,12 @@ import { - Directive, ElementRef, Input, ChangeDetectorRef, Optional, HostBinding, Inject + Directive, ElementRef, Input, ChangeDetectorRef, Optional, HostBinding, Inject, + OnDestroy } from '@angular/core'; import { IgxOverlayService } from '../../services/overlay/overlay'; import { OverlaySettings } from '../../services/public_api'; import { IgxNavigationService } from '../../core/navigation'; import { IgxToggleDirective } from '../toggle/toggle.directive'; +import { first, Subject, takeUntil } from 'rxjs'; let NEXT_ID = 0; /** @@ -26,7 +28,7 @@ let NEXT_ID = 0; selector: '[igxTooltip]', standalone: true }) -export class IgxTooltipDirective extends IgxToggleDirective { +export class IgxTooltipDirective extends IgxToggleDirective implements OnDestroy { /** * @hidden */ @@ -102,6 +104,14 @@ export class IgxTooltipDirective extends IgxToggleDirective { */ public toBeShown = false; + /** @hidden */ + public tooltipTarget; + + /** @hidden */ + public onDocumentTouchStart: (event?: Event) => void; + + private _destroy$ = new Subject(); + /** @hidden */ constructor( elementRef: ElementRef, @@ -110,6 +120,19 @@ export class IgxTooltipDirective extends IgxToggleDirective { @Optional() navigationService: IgxNavigationService) { // D.P. constructor duplication due to es6 compilation, might be obsolete in the future super(elementRef, cdr, overlayService, navigationService); + + this.opening.pipe(first(), takeUntil(this._destroy$)).subscribe(() => { + document.addEventListener('touchstart', this.onDocumentTouchStart, { passive: true }); + }); + } + + /** @hidden */ + public override ngOnDestroy() { + super.ngOnDestroy(); + + document.removeEventListener('touchstart', this.onDocumentTouchStart); + this._destroy$.next(true); + this._destroy$.complete(); } /** diff --git a/projects/igniteui-angular/src/lib/test-utils/tooltip-components.spec.ts b/projects/igniteui-angular/src/lib/test-utils/tooltip-components.spec.ts index b4d646276ec..f47c687697f 100644 --- a/projects/igniteui-angular/src/lib/test-utils/tooltip-components.spec.ts +++ b/projects/igniteui-angular/src/lib/test-utils/tooltip-components.spec.ts @@ -6,7 +6,7 @@ import { IgxToggleActionDirective, IgxToggleDirective } from '../directives/togg @Component({ template: `
dummy div for touch tests
- + @if (showButton) { From 99588c3212a6898ed2037a4d46df28f401c75447 Mon Sep 17 00:00:00 2001 From: RivaIvanova Date: Thu, 15 May 2025 17:03:21 +0300 Subject: [PATCH 2/4] test(tooltip): fix document touchstart test --- .../src/lib/directives/tooltip/tooltip.directive.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts index 76ad8f8a537..07ca6fd478f 100644 --- a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts +++ b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts @@ -577,6 +577,7 @@ describe('IgxTooltip', () => { const dummyDiv = fix.debugElement.query(By.css('.dummyDiv')); touchElement(dummyDiv); + flush(); expect(targetOne.hideTooltip).not.toHaveBeenCalled(); expect(targetTwo.hideTooltip).toHaveBeenCalledTimes(1); From 872967641e6889acc7bdbba71fb558440afa96ff Mon Sep 17 00:00:00 2001 From: RivaIvanova Date: Mon, 19 May 2025 11:36:31 +0300 Subject: [PATCH 3/4] fix(tooltip): should not emit hide event multiple times --- .../tooltip/tooltip-target.directive.ts | 7 +++++ .../tooltip/tooltip.directive.spec.ts | 26 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts index 502eabb2149..c158f9a36ce 100644 --- a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts +++ b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts @@ -211,6 +211,8 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen return; } + this.target.tooltipTarget = this; + this.checkOutletAndOutsideClick(); const shouldReturn = this.preMouseEnterCheck(); if (shouldReturn) { @@ -303,6 +305,9 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen this._overlayDefaults.closeOnEscape = true; this.target.closing.pipe(takeUntil(this.destroy$)).subscribe((event) => { + if (this.target.tooltipTarget !== this) { + return; + } const hidingArgs = { target: this, tooltip: this.target, cancel: false }; this.tooltipHide.emit(hidingArgs); @@ -335,6 +340,8 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen public showTooltip() { clearTimeout(this.target.timeoutId); + this.target.tooltipTarget = this; + if (!this.target.collapsed) { // if close animation has started finish it, or close the tooltip with no animation this.target.forceClose(this.mergedOverlaySettings); diff --git a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts index 07ca6fd478f..d51750f7576 100644 --- a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts +++ b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts @@ -582,6 +582,32 @@ describe('IgxTooltip', () => { expect(targetOne.hideTooltip).not.toHaveBeenCalled(); expect(targetTwo.hideTooltip).toHaveBeenCalledTimes(1); })); + + it('should not emit tooltipHide event multiple times', fakeAsync(() => { + spyOn(targetOne.tooltipHide, 'emit'); + spyOn(targetTwo.tooltipHide, 'emit'); + + hoverElement(buttonOne); + flush(); + + const tooltipHideArgsTargetOne = { target: targetOne, tooltip: fix.componentInstance.tooltip, cancel: false }; + const tooltipHideArgsTargetTwo = { target: targetTwo, tooltip: fix.componentInstance.tooltip, cancel: false }; + + unhoverElement(buttonOne); + tick(500); + expect(targetOne.tooltipHide.emit).toHaveBeenCalledOnceWith(tooltipHideArgsTargetOne); + expect(targetTwo.tooltipHide.emit).not.toHaveBeenCalled(); + flush(); + + hoverElement(buttonTwo); + flush(); + + unhoverElement(buttonTwo); + tick(500); + expect(targetOne.tooltipHide.emit).toHaveBeenCalledOnceWith(tooltipHideArgsTargetOne); + expect(targetTwo.tooltipHide.emit).toHaveBeenCalledOnceWith(tooltipHideArgsTargetTwo); + flush(); + })) }); describe('Tooltip integration', () => { From 4e07091f5e63d951ce89390df73dec1dafbd14a0 Mon Sep 17 00:00:00 2001 From: RivaIvanova Date: Thu, 22 May 2025 19:40:24 +0300 Subject: [PATCH 4/4] fix(tooltip): handle document touch in tooltip --- .../tooltip/tooltip-target.directive.ts | 18 ++++++------- .../tooltip/tooltip.directive.spec.ts | 4 +-- .../directives/tooltip/tooltip.directive.ts | 25 ++++++++++++------- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts index c158f9a36ce..698e2e4810f 100644 --- a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts +++ b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip-target.directive.ts @@ -211,14 +211,14 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen return; } - this.target.tooltipTarget = this; - this.checkOutletAndOutsideClick(); const shouldReturn = this.preMouseEnterCheck(); if (shouldReturn) { return; } + this.target.tooltipTarget = this; + const showingArgs = { target: this, tooltip: this.target, cancel: false }; this.tooltipShow.emit(showingArgs); @@ -265,7 +265,6 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen return; } - this.target.tooltipTarget = this; this.showTooltip(); } @@ -277,11 +276,8 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen return; } - const tooltipTarget = this.target.tooltipTarget?.nativeElement; - - if (tooltipTarget && - tooltipTarget !== event.target && - !tooltipTarget.contains(event.target) + if (this.nativeElement !== event.target && + !this.nativeElement.contains(event.target) ) { this.hideTooltip(); } @@ -308,6 +304,7 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen if (this.target.tooltipTarget !== this) { return; } + const hidingArgs = { target: this, tooltip: this.target, cancel: false }; this.tooltipHide.emit(hidingArgs); @@ -317,7 +314,6 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen }); this.nativeElement.addEventListener('touchstart', this.onTouchStart = this.onTouchStart.bind(this), { passive: true }); - this.target.onDocumentTouchStart = this.onDocumentTouchStart.bind(this); } /** @@ -340,14 +336,14 @@ export class IgxTooltipTargetDirective extends IgxToggleActionDirective implemen public showTooltip() { clearTimeout(this.target.timeoutId); - this.target.tooltipTarget = this; - if (!this.target.collapsed) { // if close animation has started finish it, or close the tooltip with no animation this.target.forceClose(this.mergedOverlaySettings); this.target.toBeHidden = false; } + this.target.tooltipTarget = this; + const showingArgs = { target: this, tooltip: this.target, cancel: false }; this.tooltipShow.emit(showingArgs); diff --git a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts index d51750f7576..5b01be0a82b 100644 --- a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts +++ b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.spec.ts @@ -579,8 +579,8 @@ describe('IgxTooltip', () => { touchElement(dummyDiv); flush(); - expect(targetOne.hideTooltip).not.toHaveBeenCalled(); - expect(targetTwo.hideTooltip).toHaveBeenCalledTimes(1); + expect(targetOne.hideTooltip).toHaveBeenCalledTimes(1); + expect(targetTwo.hideTooltip).not.toHaveBeenCalled(); })); it('should not emit tooltipHide event multiple times', fakeAsync(() => { diff --git a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.ts b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.ts index e0947820207..661606e70f5 100644 --- a/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.ts +++ b/projects/igniteui-angular/src/lib/directives/tooltip/tooltip.directive.ts @@ -1,12 +1,12 @@ import { - Directive, ElementRef, Input, ChangeDetectorRef, Optional, HostBinding, Inject, - OnDestroy + Directive, ElementRef, Input, ChangeDetectorRef, Optional, HostBinding, Inject, OnDestroy } from '@angular/core'; import { IgxOverlayService } from '../../services/overlay/overlay'; import { OverlaySettings } from '../../services/public_api'; import { IgxNavigationService } from '../../core/navigation'; import { IgxToggleDirective } from '../toggle/toggle.directive'; -import { first, Subject, takeUntil } from 'rxjs'; +import { Subject, takeUntil } from 'rxjs'; +import { IgxTooltipTargetDirective } from './tooltip-target.directive'; let NEXT_ID = 0; /** @@ -104,11 +104,10 @@ export class IgxTooltipDirective extends IgxToggleDirective implements OnDestroy */ public toBeShown = false; - /** @hidden */ - public tooltipTarget; - - /** @hidden */ - public onDocumentTouchStart: (event?: Event) => void; + /** + * @hidden + */ + public tooltipTarget: IgxTooltipTargetDirective; private _destroy$ = new Subject(); @@ -121,9 +120,13 @@ export class IgxTooltipDirective extends IgxToggleDirective implements OnDestroy // D.P. constructor duplication due to es6 compilation, might be obsolete in the future super(elementRef, cdr, overlayService, navigationService); - this.opening.pipe(first(), takeUntil(this._destroy$)).subscribe(() => { + this.onDocumentTouchStart = this.onDocumentTouchStart.bind(this); + this.overlayService.opening.pipe(takeUntil(this._destroy$)).subscribe(() => { document.addEventListener('touchstart', this.onDocumentTouchStart, { passive: true }); }); + this.overlayService.closed.pipe(takeUntil(this._destroy$)).subscribe(() => { + document.removeEventListener('touchstart', this.onDocumentTouchStart); + }); } /** @hidden */ @@ -177,4 +180,8 @@ export class IgxTooltipDirective extends IgxToggleDirective implements OnDestroy overlaySettings.positionStrategy.settings.closeAnimation = animation; } } + + private onDocumentTouchStart(event) { + this.tooltipTarget?.onDocumentTouchStart(event); + } }