From 77c37bc961ddf5f4f0530e0a10de052a512a63a4 Mon Sep 17 00:00:00 2001 From: nickonometry Date: Tue, 27 Feb 2024 10:23:03 -0500 Subject: [PATCH 01/15] feat(popover): added a delay property for the mouseenter event --- src/lib/popover/popover-constants.ts | 4 +++- src/lib/popover/popover-foundation.ts | 20 +++++++++++++++++++- src/lib/popover/popover.ts | 8 ++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/lib/popover/popover-constants.ts b/src/lib/popover/popover-constants.ts index 35bfe798c..0f4396142 100644 --- a/src/lib/popover/popover-constants.ts +++ b/src/lib/popover/popover-constants.ts @@ -10,6 +10,7 @@ const attributes = { TRIGGER_TYPE: 'trigger-type', LONGPRESS_DELAY: 'longpress-delay', PERSISTENT_HOVER: 'persistent-hover', + DELAY: 'delay', HOVER_DISMISS_DELAY: 'hover-dismiss-delay' }; @@ -31,7 +32,8 @@ const events = { }; const defaults = { - TRIGGER_TYPE: 'click' as PopoverTriggerType + TRIGGER_TYPE: 'click' as PopoverTriggerType, + DELAY: 500 }; export const POPOVER_CONSTANTS = { diff --git a/src/lib/popover/popover-foundation.ts b/src/lib/popover/popover-foundation.ts index 976eae12c..99f219dac 100644 --- a/src/lib/popover/popover-foundation.ts +++ b/src/lib/popover/popover-foundation.ts @@ -13,6 +13,7 @@ export interface IPopoverFoundation extends IOverlayAwareFoundation { longpressDelay: number; persistentHover: boolean; hoverDismissDelay: number; + delay: number; dispatchBeforeToggleEvent(state: IDismissibleStackState): boolean; } @@ -25,6 +26,7 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { private _triggerTypes: PopoverTriggerType[] = [POPOVER_CONSTANTS.defaults.TRIGGER_TYPE]; private _persistentHover = false; private _hoverDismissDelay = POPOVER_HOVER_TIMEOUT; + private _delay = POPOVER_CONSTANTS.defaults.DELAY; private _previouslyFocusedElement: HTMLElement | null = null; // Hover trigger state @@ -304,7 +306,13 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { if (!this._persistentHover) { this._adapter.addAnchorListener('mouseleave', this._anchorMouseleaveListener); } - this._openPopover(); + if (this._delay) { + window.setTimeout(() => { + this._openPopover(); + }, this._delay); + } else { + this._openPopover(); + } } } @@ -526,6 +534,16 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { } } + public get delay(): number { + return this._delay; + } + public set delay(value: number) { + if (this._delay !== value) { + this._delay = value; + this._adapter.setHostAttribute(POPOVER_CONSTANTS.attributes.DELAY, String(this._delay)); + } + } + public get hoverDismissDelay(): number { return this._hoverDismissDelay; } diff --git a/src/lib/popover/popover.ts b/src/lib/popover/popover.ts index 011c51b06..57f976521 100644 --- a/src/lib/popover/popover.ts +++ b/src/lib/popover/popover.ts @@ -16,6 +16,7 @@ export interface IPopoverComponent extends IOverlayAware, IDismissible { triggerType: PopoverTriggerType | PopoverTriggerType[]; longpressDelay: number; persistentHover: boolean; + delay: number; hoverDismissDelay: number; } @@ -99,6 +100,7 @@ export class PopoverComponent extends OverlayAware implement POPOVER_CONSTANTS.attributes.TRIGGER_TYPE, POPOVER_CONSTANTS.attributes.LONGPRESS_DELAY, POPOVER_CONSTANTS.attributes.PERSISTENT_HOVER, + POPOVER_CONSTANTS.attributes.DELAY, POPOVER_CONSTANTS.attributes.HOVER_DISMISS_DELAY ]; } @@ -138,6 +140,9 @@ export class PopoverComponent extends OverlayAware implement case POPOVER_CONSTANTS.attributes.PERSISTENT_HOVER: this.persistentHover = coerceBoolean(newValue); return; + case POPOVER_CONSTANTS.attributes.DELAY: + this.delay = coerceNumber(newValue); + return; case POPOVER_CONSTANTS.attributes.HOVER_DISMISS_DELAY: this.hoverDismissDelay = coerceNumber(newValue); return; @@ -160,6 +165,9 @@ export class PopoverComponent extends OverlayAware implement @FoundationProperty() public declare persistentHover: boolean; + @FoundationProperty() + public declare delay: number; + @FoundationProperty() public declare hoverDismissDelay: number; } From d2b23b5b6ca9febddefa4c84d7f1672ff4dd2625 Mon Sep 17 00:00:00 2001 From: nickonometry Date: Tue, 27 Feb 2024 10:42:10 -0500 Subject: [PATCH 02/15] chore(popover): added a test for hover delay --- src/lib/popover/popover-constants.ts | 2 +- src/lib/popover/popover.test.ts | 6 +++++- src/lib/tooltip/tooltip.test.ts | 11 +++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/lib/popover/popover-constants.ts b/src/lib/popover/popover-constants.ts index 0f4396142..46fff8a93 100644 --- a/src/lib/popover/popover-constants.ts +++ b/src/lib/popover/popover-constants.ts @@ -33,7 +33,7 @@ const events = { const defaults = { TRIGGER_TYPE: 'click' as PopoverTriggerType, - DELAY: 500 + DELAY: 0 }; export const POPOVER_CONSTANTS = { diff --git a/src/lib/popover/popover.test.ts b/src/lib/popover/popover.test.ts index ec58de27c..e7fc54414 100644 --- a/src/lib/popover/popover.test.ts +++ b/src/lib/popover/popover.test.ts @@ -31,6 +31,7 @@ describe('Popover', () => { expect(harness.popoverElement.triggerType).to.equal('click'); expect(harness.popoverElement.longpressDelay).to.equal(LONGPRESS_TRIGGER_DELAY); expect(harness.popoverElement.persistentHover).to.be.false; + // expect(harness.popoverElement.delay).to.equal(POPOVER_CONSTANTS.defaults.DELAY); expect(harness.popoverElement.hoverDismissDelay).to.equal(POPOVER_HOVER_TIMEOUT); }); @@ -1428,6 +1429,7 @@ interface IPopoverFixtureConfig { animationType?: PopoverAnimationType; triggerType?: PopoverTriggerType; persistentHover?: boolean; + delay?: number; } async function createFixture({ @@ -1437,7 +1439,8 @@ async function createFixture({ anchor, animationType, triggerType, - persistentHover = false + persistentHover = false, + delay }: IPopoverFixtureConfig = {}): Promise { const container = await fixture(html`
@@ -1449,6 +1452,7 @@ async function createFixture({ ?persistent=${persistent} ?arrow=${arrow} ?persistent-hover=${persistentHover} + ?delay=${delay} animation-type=${animationType ?? nothing} trigger-type=${triggerType ?? nothing}> Test popover content diff --git a/src/lib/tooltip/tooltip.test.ts b/src/lib/tooltip/tooltip.test.ts index 06ec1a1b1..709c8a720 100644 --- a/src/lib/tooltip/tooltip.test.ts +++ b/src/lib/tooltip/tooltip.test.ts @@ -434,6 +434,17 @@ describe('Tooltip', () => { expect(harness.isOpen).to.be.true; }); + it('should open when hovering the trigger button and a delay is set', async () => { + const harness = await createFixture({ triggerType: 'hover', delay: 500 }); + + expect(harness.isOpen).to.be.false; + + await harness.hoverTrigger(); + await timer(harness.tooltipElement.delay); + + expect(harness.isOpen).to.be.true; + }); + it('should open and close when hovering and unhovering the trigger button', async () => { const harness = await createFixture({ triggerType: 'hover' }); From 691479531f0f23740a97c4aa4c1db0ced11e7424 Mon Sep 17 00:00:00 2001 From: nickonometry Date: Tue, 27 Feb 2024 10:48:39 -0500 Subject: [PATCH 03/15] chore(popover): added an ejs demo control for delay --- src/dev/pages/popover/popover.html | 1 + src/dev/pages/popover/popover.ts | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/dev/pages/popover/popover.html b/src/dev/pages/popover/popover.html index 23c7800f2..b6d41a1a4 100644 --- a/src/dev/pages/popover/popover.html +++ b/src/dev/pages/popover/popover.html @@ -4,6 +4,7 @@ title: 'Popover', includePath: './pages/popover/popover.ejs', options: [ + { type: 'text-field', inputType: 'number', id: 'opt-delay', label: 'Delay', defaultValue: 0 }, { type: 'select', label: 'Placement', diff --git a/src/dev/pages/popover/popover.ts b/src/dev/pages/popover/popover.ts index 3bb75048f..78cce3ce7 100644 --- a/src/dev/pages/popover/popover.ts +++ b/src/dev/pages/popover/popover.ts @@ -10,6 +10,7 @@ import '@tylertech/forge/checkbox'; import '@tylertech/forge/label'; import './popover.scss'; +const delayInput = document.querySelector('#opt-delay') as HTMLInputElement; const popover = document.querySelector('#my-popover') as IPopoverComponent; const showPopoverButton = document.querySelector('#popover-trigger') as HTMLButtonElement; const closeButton = document.querySelector('#close-button') as HTMLButtonElement; @@ -17,6 +18,10 @@ const clippingContainer = document.querySelector('.clipping-container') as HTMLE const preventCloseToggle = document.querySelector('#opt-prevent-close') as ISwitchComponent; const richTooltipPopover = document.querySelector('#rich-tooltip-popover') as IPopoverComponent; +delayInput.addEventListener('input', (e) => { + popover.delay = Number(delayInput.value); +}); + popover.addEventListener('forge-popover-beforetoggle', (evt: CustomEvent) => { console.log('forge-popover-beforetoggle', evt.detail); if (preventCloseToggle.on && evt.detail.newState === 'closed' && evt.cancelable) { From c165bcc3808347387bd34235fbfe98b6114ea6a6 Mon Sep 17 00:00:00 2001 From: nickonometry Date: Tue, 27 Feb 2024 10:52:55 -0500 Subject: [PATCH 04/15] chore(popover): added delay to the defaults value test --- src/lib/popover/popover.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/popover/popover.test.ts b/src/lib/popover/popover.test.ts index e7fc54414..42b6212c0 100644 --- a/src/lib/popover/popover.test.ts +++ b/src/lib/popover/popover.test.ts @@ -31,7 +31,7 @@ describe('Popover', () => { expect(harness.popoverElement.triggerType).to.equal('click'); expect(harness.popoverElement.longpressDelay).to.equal(LONGPRESS_TRIGGER_DELAY); expect(harness.popoverElement.persistentHover).to.be.false; - // expect(harness.popoverElement.delay).to.equal(POPOVER_CONSTANTS.defaults.DELAY); + expect(harness.popoverElement.delay).to.equal(POPOVER_CONSTANTS.defaults.DELAY); expect(harness.popoverElement.hoverDismissDelay).to.equal(POPOVER_HOVER_TIMEOUT); }); From 69d28a64b6efb5470e7f9df4a5e907d3f030bb0f Mon Sep 17 00:00:00 2001 From: nickonometry Date: Wed, 28 Feb 2024 10:44:48 -0500 Subject: [PATCH 05/15] chore(popover): added hoverTimeout var and added in clearTimeout where needed --- src/lib/popover/popover-foundation.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lib/popover/popover-foundation.ts b/src/lib/popover/popover-foundation.ts index 99f219dac..e4cacf549 100644 --- a/src/lib/popover/popover-foundation.ts +++ b/src/lib/popover/popover-foundation.ts @@ -33,6 +33,7 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { private _hoverAnchorLeaveTimeout: undefined | number; private _popoverMouseleaveTimeout: undefined | number; private _currentHoverCoords: undefined | { x: number; y: number }; + private _hoverTimeout: number | undefined; // Click trigger listeners private _anchorClickListener = this._onAnchorClick.bind(this); @@ -265,7 +266,7 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { return; } } - + window.clearTimeout(this._hoverTimeout); this._tryRemoveHoverListeners(); this._requestClose('hover'); } @@ -307,8 +308,8 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { this._adapter.addAnchorListener('mouseleave', this._anchorMouseleaveListener); } if (this._delay) { - window.setTimeout(() => { - this._openPopover(); + this._hoverTimeout = window.setTimeout(() => { + this._openPopover(); }, this._delay); } else { this._openPopover(); @@ -325,6 +326,7 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { */ private _onAnchorMouseleave(): void { this._startHoverListeners(); + window.clearTimeout(this._hoverTimeout); this._hoverAnchorLeaveTimeout = window.setTimeout(() => { this._hoverAnchorLeaveTimeout = undefined; From 96b4915453eb77316f28c35d14ffff57521c417d Mon Sep 17 00:00:00 2001 From: nickonometry Date: Wed, 28 Feb 2024 10:46:31 -0500 Subject: [PATCH 06/15] chore(popover): added comment docs for the delay property and attribute --- src/lib/popover/popover.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/popover/popover.ts b/src/lib/popover/popover.ts index 57f976521..22647b16f 100644 --- a/src/lib/popover/popover.ts +++ b/src/lib/popover/popover.ts @@ -42,6 +42,7 @@ declare global { * @property {number} longpressDelay - The delay in milliseconds before a longpress event is detected. * @property {boolean} persistentHover - Whether or not the popover should remain open when the user hovers outside the popover. * @property {number} hoverDismissDelay - The delay in milliseconds before the popover is dismissed when the user hovers outside of the popover. + * @property {number} delay - The delay in milliseconds before the popover is shown. * * @attribute {string} arrow - Whether or not the popover should render an arrow. * @attribute {string} animation-type - The animation type to use for the popover. Valid values are `'none'`, `'fade'`, `'slide'`, and `'zoom'` (default). @@ -49,6 +50,7 @@ declare global { * @attribute {string} longpress-delay - The delay in milliseconds before a longpress event is detected. * @attribute {string} persistent-hover - Whether or not the popover should remain open when the user hovers outside the popover. * @attribute {string} hover-dismiss-delay - The delay in milliseconds before the popover is dismissed when the user hovers outside of the popover. + * @attribute {number} delay - The delay in milliseconds before the popover is shown. * * @event {CustomEvent Date: Wed, 28 Feb 2024 11:08:41 -0500 Subject: [PATCH 07/15] chore(popover): changed the property/attribute name to hoverDelay --- src/dev/pages/popover/popover.ts | 4 +--- src/lib/popover/popover-constants.ts | 4 ++-- src/lib/popover/popover-foundation.ts | 20 ++++++++++---------- src/lib/popover/popover.ts | 12 ++++++------ 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/dev/pages/popover/popover.ts b/src/dev/pages/popover/popover.ts index 78cce3ce7..42ccd8d4c 100644 --- a/src/dev/pages/popover/popover.ts +++ b/src/dev/pages/popover/popover.ts @@ -18,9 +18,7 @@ const clippingContainer = document.querySelector('.clipping-container') as HTMLE const preventCloseToggle = document.querySelector('#opt-prevent-close') as ISwitchComponent; const richTooltipPopover = document.querySelector('#rich-tooltip-popover') as IPopoverComponent; -delayInput.addEventListener('input', (e) => { - popover.delay = Number(delayInput.value); -}); +delayInput.addEventListener('input', (e) => popover.hoverDelay = Number(delayInput.value)); popover.addEventListener('forge-popover-beforetoggle', (evt: CustomEvent) => { console.log('forge-popover-beforetoggle', evt.detail); diff --git a/src/lib/popover/popover-constants.ts b/src/lib/popover/popover-constants.ts index 46fff8a93..489e434dc 100644 --- a/src/lib/popover/popover-constants.ts +++ b/src/lib/popover/popover-constants.ts @@ -10,7 +10,7 @@ const attributes = { TRIGGER_TYPE: 'trigger-type', LONGPRESS_DELAY: 'longpress-delay', PERSISTENT_HOVER: 'persistent-hover', - DELAY: 'delay', + HOVER_DELAY: 'hover-delay', HOVER_DISMISS_DELAY: 'hover-dismiss-delay' }; @@ -33,7 +33,7 @@ const events = { const defaults = { TRIGGER_TYPE: 'click' as PopoverTriggerType, - DELAY: 0 + HOVER_DELAY: 0 }; export const POPOVER_CONSTANTS = { diff --git a/src/lib/popover/popover-foundation.ts b/src/lib/popover/popover-foundation.ts index e4cacf549..e80401cab 100644 --- a/src/lib/popover/popover-foundation.ts +++ b/src/lib/popover/popover-foundation.ts @@ -13,7 +13,7 @@ export interface IPopoverFoundation extends IOverlayAwareFoundation { longpressDelay: number; persistentHover: boolean; hoverDismissDelay: number; - delay: number; + hoverDelay: number; dispatchBeforeToggleEvent(state: IDismissibleStackState): boolean; } @@ -26,7 +26,7 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { private _triggerTypes: PopoverTriggerType[] = [POPOVER_CONSTANTS.defaults.TRIGGER_TYPE]; private _persistentHover = false; private _hoverDismissDelay = POPOVER_HOVER_TIMEOUT; - private _delay = POPOVER_CONSTANTS.defaults.DELAY; + private _hoverDelay = POPOVER_CONSTANTS.defaults.HOVER_DELAY; private _previouslyFocusedElement: HTMLElement | null = null; // Hover trigger state @@ -307,10 +307,10 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { if (!this._persistentHover) { this._adapter.addAnchorListener('mouseleave', this._anchorMouseleaveListener); } - if (this._delay) { + if (this._hoverDelay) { this._hoverTimeout = window.setTimeout(() => { this._openPopover(); - }, this._delay); + }, this._hoverDelay); } else { this._openPopover(); } @@ -536,13 +536,13 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { } } - public get delay(): number { - return this._delay; + public get hoverDelay(): number { + return this._hoverDelay; } - public set delay(value: number) { - if (this._delay !== value) { - this._delay = value; - this._adapter.setHostAttribute(POPOVER_CONSTANTS.attributes.DELAY, String(this._delay)); + public set hoverDelay(value: number) { + if (this._hoverDelay !== value) { + this._hoverDelay = value; + this._adapter.setHostAttribute(POPOVER_CONSTANTS.attributes.HOVER_DELAY, String(this._hoverDelay)); } } diff --git a/src/lib/popover/popover.ts b/src/lib/popover/popover.ts index 22647b16f..238a3c264 100644 --- a/src/lib/popover/popover.ts +++ b/src/lib/popover/popover.ts @@ -16,7 +16,7 @@ export interface IPopoverComponent extends IOverlayAware, IDismissible { triggerType: PopoverTriggerType | PopoverTriggerType[]; longpressDelay: number; persistentHover: boolean; - delay: number; + hoverDelay: number; hoverDismissDelay: number; } @@ -42,7 +42,7 @@ declare global { * @property {number} longpressDelay - The delay in milliseconds before a longpress event is detected. * @property {boolean} persistentHover - Whether or not the popover should remain open when the user hovers outside the popover. * @property {number} hoverDismissDelay - The delay in milliseconds before the popover is dismissed when the user hovers outside of the popover. - * @property {number} delay - The delay in milliseconds before the popover is shown. + * @property {number} hoverDelay - The delay in milliseconds before the popover is shown. * * @attribute {string} arrow - Whether or not the popover should render an arrow. * @attribute {string} animation-type - The animation type to use for the popover. Valid values are `'none'`, `'fade'`, `'slide'`, and `'zoom'` (default). @@ -50,7 +50,7 @@ declare global { * @attribute {string} longpress-delay - The delay in milliseconds before a longpress event is detected. * @attribute {string} persistent-hover - Whether or not the popover should remain open when the user hovers outside the popover. * @attribute {string} hover-dismiss-delay - The delay in milliseconds before the popover is dismissed when the user hovers outside of the popover. - * @attribute {number} delay - The delay in milliseconds before the popover is shown. + * @attribute {number} hover-delay - The delay in milliseconds before the popover is shown. * * @event {CustomEvent implement POPOVER_CONSTANTS.attributes.TRIGGER_TYPE, POPOVER_CONSTANTS.attributes.LONGPRESS_DELAY, POPOVER_CONSTANTS.attributes.PERSISTENT_HOVER, - POPOVER_CONSTANTS.attributes.DELAY, + POPOVER_CONSTANTS.attributes.HOVER_DELAY, POPOVER_CONSTANTS.attributes.HOVER_DISMISS_DELAY ]; } @@ -142,7 +142,7 @@ export class PopoverComponent extends OverlayAware implement case POPOVER_CONSTANTS.attributes.PERSISTENT_HOVER: this.persistentHover = coerceBoolean(newValue); return; - case POPOVER_CONSTANTS.attributes.DELAY: + case POPOVER_CONSTANTS.attributes.HOVER_DELAY: this.delay = coerceNumber(newValue); return; case POPOVER_CONSTANTS.attributes.HOVER_DISMISS_DELAY: @@ -168,7 +168,7 @@ export class PopoverComponent extends OverlayAware implement public declare persistentHover: boolean; @FoundationProperty() - public declare delay: number; + public declare hoverDelay: number; @FoundationProperty() public declare hoverDismissDelay: number; From e5853453b94d106593582b37ecf14b50618d2ac3 Mon Sep 17 00:00:00 2001 From: nickonometry Date: Wed, 28 Feb 2024 11:21:48 -0500 Subject: [PATCH 08/15] chore(popover): added a test for the hoverDelay property --- src/lib/popover/popover.test.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/lib/popover/popover.test.ts b/src/lib/popover/popover.test.ts index 42b6212c0..7650cf8ee 100644 --- a/src/lib/popover/popover.test.ts +++ b/src/lib/popover/popover.test.ts @@ -31,7 +31,7 @@ describe('Popover', () => { expect(harness.popoverElement.triggerType).to.equal('click'); expect(harness.popoverElement.longpressDelay).to.equal(LONGPRESS_TRIGGER_DELAY); expect(harness.popoverElement.persistentHover).to.be.false; - expect(harness.popoverElement.delay).to.equal(POPOVER_CONSTANTS.defaults.DELAY); + expect(harness.popoverElement.hoverDelay).to.equal(POPOVER_CONSTANTS.defaults.HOVER_DELAY); expect(harness.popoverElement.hoverDismissDelay).to.equal(POPOVER_HOVER_TIMEOUT); }); @@ -700,6 +700,17 @@ describe('Popover', () => { expect(harness.isOpen).to.be.false; }); + it('should open with a delay when hovering over the trigger button and a delay is set', async () => { + const harness = await createFixture({ triggerType: 'hover', hoverDelay: 500 }); + + expect(harness.isOpen).to.be.false; + + await harness.hoverTrigger(); + await timer(harness.popoverElement.hoverDelay); + + expect(harness.isOpen).to.be.true; + }); + it('should not close if persistent hover is enabled', async () => { const harness = await createFixture({ triggerType: 'hover', persistentHover: true }); @@ -1429,7 +1440,7 @@ interface IPopoverFixtureConfig { animationType?: PopoverAnimationType; triggerType?: PopoverTriggerType; persistentHover?: boolean; - delay?: number; + hoverDelay?: number; } async function createFixture({ @@ -1440,7 +1451,7 @@ async function createFixture({ animationType, triggerType, persistentHover = false, - delay + hoverDelay }: IPopoverFixtureConfig = {}): Promise { const container = await fixture(html`
@@ -1452,7 +1463,7 @@ async function createFixture({ ?persistent=${persistent} ?arrow=${arrow} ?persistent-hover=${persistentHover} - ?delay=${delay} + ?hoverDelay=${hoverDelay} animation-type=${animationType ?? nothing} trigger-type=${triggerType ?? nothing}> Test popover content From 46159c5f233bc04a4f7fa7d1be8a5e5fc799ddab Mon Sep 17 00:00:00 2001 From: nickonometry Date: Wed, 28 Feb 2024 11:24:44 -0500 Subject: [PATCH 09/15] chore(popover): fixing issues brought up in PR comments --- src/dev/pages/popover/popover.html | 2 +- src/dev/pages/popover/popover.ts | 2 +- src/lib/popover/popover-foundation.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dev/pages/popover/popover.html b/src/dev/pages/popover/popover.html index b6d41a1a4..31013e015 100644 --- a/src/dev/pages/popover/popover.html +++ b/src/dev/pages/popover/popover.html @@ -4,7 +4,7 @@ title: 'Popover', includePath: './pages/popover/popover.ejs', options: [ - { type: 'text-field', inputType: 'number', id: 'opt-delay', label: 'Delay', defaultValue: 0 }, + { type: 'text-field', inputType: 'number', id: 'opt-hover-delay', label: 'Hover Delay', defaultValue: 0 }, { type: 'select', label: 'Placement', diff --git a/src/dev/pages/popover/popover.ts b/src/dev/pages/popover/popover.ts index 42ccd8d4c..9e9dbb09e 100644 --- a/src/dev/pages/popover/popover.ts +++ b/src/dev/pages/popover/popover.ts @@ -10,7 +10,7 @@ import '@tylertech/forge/checkbox'; import '@tylertech/forge/label'; import './popover.scss'; -const delayInput = document.querySelector('#opt-delay') as HTMLInputElement; +const delayInput = document.querySelector('#opt-hover-delay') as HTMLInputElement; const popover = document.querySelector('#my-popover') as IPopoverComponent; const showPopoverButton = document.querySelector('#popover-trigger') as HTMLButtonElement; const closeButton = document.querySelector('#close-button') as HTMLButtonElement; diff --git a/src/lib/popover/popover-foundation.ts b/src/lib/popover/popover-foundation.ts index e80401cab..9cc88e7a9 100644 --- a/src/lib/popover/popover-foundation.ts +++ b/src/lib/popover/popover-foundation.ts @@ -309,7 +309,7 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { } if (this._hoverDelay) { this._hoverTimeout = window.setTimeout(() => { - this._openPopover(); + this._openPopover(); }, this._hoverDelay); } else { this._openPopover(); From 3b925160005fa33d8c8d5949972b1d7ee84a7887 Mon Sep 17 00:00:00 2001 From: nickonometry Date: Wed, 28 Feb 2024 11:51:13 -0500 Subject: [PATCH 10/15] chore(popover): added new tests for NaN and < 0 --- src/lib/popover/popover-foundation.ts | 3 +++ src/lib/popover/popover.test.ts | 24 +++++++++++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/lib/popover/popover-foundation.ts b/src/lib/popover/popover-foundation.ts index 9cc88e7a9..7aea0efd3 100644 --- a/src/lib/popover/popover-foundation.ts +++ b/src/lib/popover/popover-foundation.ts @@ -540,6 +540,9 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { return this._hoverDelay; } public set hoverDelay(value: number) { + if (isNaN(value) || value < 0) { + value = POPOVER_CONSTANTS.defaults.HOVER_DELAY; + } if (this._hoverDelay !== value) { this._hoverDelay = value; this._adapter.setHostAttribute(POPOVER_CONSTANTS.attributes.HOVER_DELAY, String(this._hoverDelay)); diff --git a/src/lib/popover/popover.test.ts b/src/lib/popover/popover.test.ts index 7650cf8ee..d057caf56 100644 --- a/src/lib/popover/popover.test.ts +++ b/src/lib/popover/popover.test.ts @@ -701,15 +701,25 @@ describe('Popover', () => { }); it('should open with a delay when hovering over the trigger button and a delay is set', async () => { - const harness = await createFixture({ triggerType: 'hover', hoverDelay: 500 }); + const harness = await createFixture({ triggerType: 'hover', hoverDelay: 500 }); - expect(harness.isOpen).to.be.false; + expect(harness.isOpen).to.be.false; + + await harness.hoverTrigger(); + await timer(harness.popoverElement.hoverDelay); + + expect(harness.isOpen).to.be.true; + }); - await harness.hoverTrigger(); - await timer(harness.popoverElement.hoverDelay); + it('should set the default hoverDelay value if NaN', async () => { + const harness = await createFixture({ triggerType: 'hover', hoverDelay: 'Testing' }); + expect(harness.popoverElement.hoverDelay).to.equal(0); + }); - expect(harness.isOpen).to.be.true; - }); + it('should set the default hoverDelay value if the hoverDelay < 0', async () => { + const harness = await createFixture({ triggerType: 'hover', hoverDelay: -400 }); + expect(harness.popoverElement.hoverDelay).to.equal(0); + }); it('should not close if persistent hover is enabled', async () => { const harness = await createFixture({ triggerType: 'hover', persistentHover: true }); @@ -1440,7 +1450,7 @@ interface IPopoverFixtureConfig { animationType?: PopoverAnimationType; triggerType?: PopoverTriggerType; persistentHover?: boolean; - hoverDelay?: number; + hoverDelay?: any; } async function createFixture({ From 4dad0b7767421255ebb32125d1a70a96a79e7b17 Mon Sep 17 00:00:00 2001 From: nickonometry Date: Wed, 28 Feb 2024 11:56:44 -0500 Subject: [PATCH 11/15] chore(tooltip): removed duplicate test --- src/lib/tooltip/tooltip.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/lib/tooltip/tooltip.test.ts b/src/lib/tooltip/tooltip.test.ts index 709c8a720..06ec1a1b1 100644 --- a/src/lib/tooltip/tooltip.test.ts +++ b/src/lib/tooltip/tooltip.test.ts @@ -434,17 +434,6 @@ describe('Tooltip', () => { expect(harness.isOpen).to.be.true; }); - it('should open when hovering the trigger button and a delay is set', async () => { - const harness = await createFixture({ triggerType: 'hover', delay: 500 }); - - expect(harness.isOpen).to.be.false; - - await harness.hoverTrigger(); - await timer(harness.tooltipElement.delay); - - expect(harness.isOpen).to.be.true; - }); - it('should open and close when hovering and unhovering the trigger button', async () => { const harness = await createFixture({ triggerType: 'hover' }); From 6d5dd545aafeaef9bd8884215100649b5783d00a Mon Sep 17 00:00:00 2001 From: nickonometry Date: Wed, 28 Feb 2024 12:03:07 -0500 Subject: [PATCH 12/15] chore(popover): updated typings issue in the popover test file --- src/lib/popover/popover.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/popover/popover.test.ts b/src/lib/popover/popover.test.ts index d057caf56..8037f4117 100644 --- a/src/lib/popover/popover.test.ts +++ b/src/lib/popover/popover.test.ts @@ -712,7 +712,7 @@ describe('Popover', () => { }); it('should set the default hoverDelay value if NaN', async () => { - const harness = await createFixture({ triggerType: 'hover', hoverDelay: 'Testing' }); + const harness = await createFixture({ triggerType: 'hover', hoverDelay: 'Testing' as any }); expect(harness.popoverElement.hoverDelay).to.equal(0); }); @@ -1450,7 +1450,7 @@ interface IPopoverFixtureConfig { animationType?: PopoverAnimationType; triggerType?: PopoverTriggerType; persistentHover?: boolean; - hoverDelay?: any; + hoverDelay?: number; } async function createFixture({ From 8e675927fee51fe407cd0809f5490f1228e5f29c Mon Sep 17 00:00:00 2001 From: nickonometry Date: Wed, 28 Feb 2024 12:06:04 -0500 Subject: [PATCH 13/15] chore(popover): fixed a variable name issue causing a build error --- src/lib/popover/popover.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/popover/popover.ts b/src/lib/popover/popover.ts index 238a3c264..cd73a10d3 100644 --- a/src/lib/popover/popover.ts +++ b/src/lib/popover/popover.ts @@ -143,7 +143,7 @@ export class PopoverComponent extends OverlayAware implement this.persistentHover = coerceBoolean(newValue); return; case POPOVER_CONSTANTS.attributes.HOVER_DELAY: - this.delay = coerceNumber(newValue); + this.hoverDelay = coerceNumber(newValue); return; case POPOVER_CONSTANTS.attributes.HOVER_DISMISS_DELAY: this.hoverDismissDelay = coerceNumber(newValue); From aaf7d6316edb6791ccfc7f0663d51ce98ac1bbbf Mon Sep 17 00:00:00 2001 From: "Nichols, Kieran" Date: Wed, 28 Feb 2024 12:17:33 -0500 Subject: [PATCH 14/15] chore(popover): ensure all timeouts are cleared when element is disconnected --- src/lib/popover/popover-foundation.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lib/popover/popover-foundation.ts b/src/lib/popover/popover-foundation.ts index 7aea0efd3..bd0b4b1f5 100644 --- a/src/lib/popover/popover-foundation.ts +++ b/src/lib/popover/popover-foundation.ts @@ -72,6 +72,11 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { public override destroy(): void { super.destroy(); + + window.clearTimeout(this._hoverTimeout); + window.clearTimeout(this._hoverAnchorLeaveTimeout); + window.clearTimeout(this._popoverMouseleaveTimeout); + this._previouslyFocusedElement = null; if (this.open) { @@ -309,8 +314,8 @@ export class PopoverFoundation extends BaseClass implements IPopoverFoundation { } if (this._hoverDelay) { this._hoverTimeout = window.setTimeout(() => { - this._openPopover(); - }, this._hoverDelay); + this._openPopover(); + }, this._hoverDelay); } else { this._openPopover(); } From cd6198e1ce7af1883a87bd3c973808561e7a5725 Mon Sep 17 00:00:00 2001 From: "Nichols, Kieran" Date: Wed, 28 Feb 2024 12:37:39 -0500 Subject: [PATCH 15/15] chore: update test to avoid race condition --- src/lib/popover/popover.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/popover/popover.test.ts b/src/lib/popover/popover.test.ts index 8037f4117..da9492449 100644 --- a/src/lib/popover/popover.test.ts +++ b/src/lib/popover/popover.test.ts @@ -706,18 +706,20 @@ describe('Popover', () => { expect(harness.isOpen).to.be.false; await harness.hoverTrigger(); - await timer(harness.popoverElement.hoverDelay); + await timer(harness.popoverElement.hoverDelay + 100); expect(harness.isOpen).to.be.true; }); it('should set the default hoverDelay value if NaN', async () => { const harness = await createFixture({ triggerType: 'hover', hoverDelay: 'Testing' as any }); + expect(harness.popoverElement.hoverDelay).to.equal(0); }); it('should set the default hoverDelay value if the hoverDelay < 0', async () => { const harness = await createFixture({ triggerType: 'hover', hoverDelay: -400 }); + expect(harness.popoverElement.hoverDelay).to.equal(0); });