From 6a33f8da9d3e66a3e50e4b5ada44e6822eef6ee1 Mon Sep 17 00:00:00 2001 From: Mark Otto Date: Wed, 12 Aug 2026 11:52:43 -0700 Subject: [PATCH 1/3] fix(popover): keep dismiss-on-next-click open inside the tip Selecting text or focusing a control inside a focus-triggered popover closed it, because focus left the trigger. Keep the tip open for tip focus and tip pointerdown, and dismiss on outside pointerdown instead. Fixes #31088 --- js/src/tooltip.ts | 92 +++++++++- js/tests/unit/popover.spec.js | 181 +++++++++++++++++++ site/src/content/docs/components/popover.mdx | 3 +- 3 files changed, 271 insertions(+), 5 deletions(-) diff --git a/js/src/tooltip.ts b/js/src/tooltip.ts index e357d9af2ffe..ec327e74b3ab 100644 --- a/js/src/tooltip.ts +++ b/js/src/tooltip.ts @@ -70,6 +70,8 @@ const EVENT_FOCUSIN = 'focusin' const EVENT_FOCUSOUT = 'focusout' const EVENT_MOUSEENTER = 'mouseenter' const EVENT_MOUSELEAVE = 'mouseleave' +const EVENT_POINTERDOWN = 'pointerdown' +const EVENT_POINTERUP = 'pointerup' const EVENT_KEYDOWN = 'keydown' const AttachmentMap: Record = { @@ -162,6 +164,9 @@ class Tooltip extends BaseComponent { protected declare _floatingCleanup: (() => void) | null protected declare _keydownHandler: ((event: KeyboardEvent) => void) | null protected declare _tipEventOut: ((event: BootstrapEvent) => void) | null + protected declare _outsidePointerHandler: ((event: PointerEvent) => void) | null + protected declare _tipPointerUpHandler: (() => void) | null + protected declare _tipPointerDown: boolean protected declare _templateFactory: TemplateFactory | null protected declare _newContent: Record | null protected declare _mediaQueryListeners: BreakpointListener[] @@ -185,6 +190,9 @@ class Tooltip extends BaseComponent { this._floatingCleanup = null this._keydownHandler = null this._tipEventOut = null + this._outsidePointerHandler = null + this._tipPointerUpHandler = null + this._tipPointerDown = false this._templateFactory = null this._newContent = null this._mediaQueryListeners = [] @@ -239,6 +247,7 @@ class Tooltip extends BaseComponent { this._clearTimeout() this._removeEscapeListener() + this._removeFocusOutsideListener() EventHandler.off(this._element.closest(SELECTOR_MODAL), EVENT_MODAL_HIDE, this._hideModalHandler) @@ -297,6 +306,7 @@ class Tooltip extends BaseComponent { // Allow dismissing the tooltip with the Escape key (WCAG 1.4.13) this._setEscapeListener() + this._setFocusOutsideListener() // If this is a touch-enabled device we add extra // empty mouseover listeners to the body's immediate children; @@ -332,6 +342,7 @@ class Tooltip extends BaseComponent { } this._removeEscapeListener() + this._removeFocusOutsideListener() const tip = this._getTipElement() tip.classList.remove(CLASS_NAME_SHOW) @@ -408,6 +419,8 @@ class Tooltip extends BaseComponent { tip.classList.add(this._getInstantClassName()) } + this._setFocusTipListeners(tip) + return tip } @@ -657,8 +670,13 @@ class Tooltip extends BaseComponent { }) EventHandler.on(this._element, eventOut, this._config.selector as string, event => { const context = this._initializeOnDelegatedTarget(event) - context._activeTrigger[event.type === 'focusout' ? TRIGGER_FOCUS : TRIGGER_HOVER] = - context._isInside(event.relatedTarget) + + if (event.type === 'focusout') { + context._activeTrigger[TRIGGER_FOCUS] = + context._isInside(event.relatedTarget) || context._tipPointerDown + } else { + context._activeTrigger[TRIGGER_HOVER] = context._isInside(event.relatedTarget) + } context._leave() }) @@ -714,8 +732,9 @@ class Tooltip extends BaseComponent { this._tipEventOut = null } - protected _isInside(element: Node | null): boolean { - return this._element.contains(element) || Boolean(this.tip?.contains(element)) + protected _isInside(target: EventTarget | null): boolean { + return target instanceof Node && + (this._element.contains(target) || Boolean(this.tip?.contains(target))) } protected _getTrigger(): string { @@ -737,6 +756,70 @@ class Tooltip extends BaseComponent { return events[trigger] } + protected _hasFocusTrigger(): boolean { + return this._getTrigger().split(' ').includes(TRIGGER_FOCUS) + } + + protected _setFocusTipListeners(tip: HTMLElement): void { + if (!this._hasFocusTrigger()) { + return + } + + EventHandler.on(tip, this.constructor.eventName(EVENT_POINTERDOWN), () => { + this._tipPointerDown = true + this._activeTrigger[TRIGGER_FOCUS] = true + }) + + EventHandler.on(tip, this.constructor.eventName(EVENT_FOCUSIN), () => { + this._activeTrigger[TRIGGER_FOCUS] = true + }) + + EventHandler.on(tip, this.constructor.eventName(EVENT_FOCUSOUT), (event: BootstrapEvent) => { + this._activeTrigger[TRIGGER_FOCUS] = + this._isInside(event.relatedTarget) || this._tipPointerDown + this._leave() + }) + } + + protected _setFocusOutsideListener(): void { + if (this._outsidePointerHandler || !this._hasFocusTrigger()) { + return + } + + this._tipPointerUpHandler = () => { + this._tipPointerDown = false + } + + this._outsidePointerHandler = event => { + if (!this._isShown() || this._isInside(event.target)) { + return + } + + this._activeTrigger[TRIGGER_FOCUS] = false + this.hide() + } + + const doc = this._element.ownerDocument + doc.addEventListener(EVENT_POINTERUP, this._tipPointerUpHandler, true) + doc.addEventListener(EVENT_POINTERDOWN, this._outsidePointerHandler, true) + } + + protected _removeFocusOutsideListener(): void { + const doc = this._element?.ownerDocument + + if (this._tipPointerUpHandler && doc) { + doc.removeEventListener(EVENT_POINTERUP, this._tipPointerUpHandler, true) + } + + if (this._outsidePointerHandler && doc) { + doc.removeEventListener(EVENT_POINTERDOWN, this._outsidePointerHandler, true) + } + + this._tipPointerUpHandler = null + this._outsidePointerHandler = null + this._tipPointerDown = false + } + protected _setEscapeListener(): void { if (this._keydownHandler) { return @@ -915,6 +998,7 @@ class Tooltip extends BaseComponent { if (this.tip) { this._removeTipListeners(this.tip) + EventHandler.off(this.tip, this.constructor.EVENT_KEY) this.tip.remove() this.tip = null } diff --git a/js/tests/unit/popover.spec.js b/js/tests/unit/popover.spec.js index 28ed6c304b17..9c0991fcd221 100644 --- a/js/tests/unit/popover.spec.js +++ b/js/tests/unit/popover.spec.js @@ -457,6 +457,187 @@ describe('Popover', () => { }) }) + describe('dismiss on next click', () => { + it('should not hide when focus moves into the popover', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = 'Dismissible' + + const popoverEl = fixtureEl.querySelector('a') + const popover = new Popover(popoverEl, { + trigger: 'focus', + html: true, + content: 'Inside' + }) + + popoverEl.addEventListener('shown.bs.popover', () => { + const tip = document.querySelector('.popover') + const insideLink = tip.querySelector('.inside-link') + const leaveSpy = spyOn(popover, '_leave').and.callThrough() + + popoverEl.dispatchEvent(new FocusEvent('focusout', { + bubbles: true, + relatedTarget: insideLink + })) + + expect(leaveSpy).toHaveBeenCalled() + expect(popover._activeTrigger.focus).toBeTrue() + expect(tip).toHaveClass('show') + resolve() + }) + + popoverEl.dispatchEvent(createEvent('focusin')) + }) + }) + + it('should hide when focus leaves the trigger for outside content', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = 'Dismissible' + + const popoverEl = fixtureEl.querySelector('a') + // eslint-disable-next-line no-new + new Popover(popoverEl, { + trigger: 'focus', + content: 'Selectable text' + }) + + popoverEl.addEventListener('shown.bs.popover', () => { + popoverEl.addEventListener('hidden.bs.popover', () => { + expect(document.querySelector('.popover')).toBeNull() + resolve() + }) + + popoverEl.dispatchEvent(new FocusEvent('focusout', { + bubbles: true, + relatedTarget: document.body + })) + }) + + popoverEl.dispatchEvent(createEvent('focusin')) + }) + }) + + it('should stay open when the pointer presses inside the tip', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = 'Dismissible' + + const popoverEl = fixtureEl.querySelector('a') + const popover = new Popover(popoverEl, { + trigger: 'focus', + content: 'Selectable text' + }) + + popoverEl.addEventListener('shown.bs.popover', () => { + const tip = document.querySelector('.popover') + const body = tip.querySelector('.popover-body') + + body.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) + popoverEl.dispatchEvent(new FocusEvent('focusout', { + bubbles: true, + relatedTarget: null + })) + + expect(popover._activeTrigger.focus).toBeTrue() + expect(tip).toHaveClass('show') + resolve() + }) + + popoverEl.dispatchEvent(createEvent('focusin')) + }) + }) + + it('should dismiss on pointerdown outside the tip after tip interaction', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = 'Dismissible' + + const popoverEl = fixtureEl.querySelector('a') + // eslint-disable-next-line no-new + new Popover(popoverEl, { + trigger: 'focus', + content: 'Selectable text' + }) + + popoverEl.addEventListener('shown.bs.popover', () => { + const tip = document.querySelector('.popover') + const body = tip.querySelector('.popover-body') + + popoverEl.addEventListener('hidden.bs.popover', () => { + expect(document.querySelector('.popover')).toBeNull() + resolve() + }) + + body.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) + popoverEl.dispatchEvent(new FocusEvent('focusout', { + bubbles: true, + relatedTarget: null + })) + document.dispatchEvent(new PointerEvent('pointerup', { bubbles: true })) + document.body.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) + }) + + popoverEl.dispatchEvent(createEvent('focusin')) + }) + }) + + it('should allow focusable tip content to receive focus without dismissing', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = 'Dismissible' + + const popoverEl = fixtureEl.querySelector('a') + const popover = new Popover(popoverEl, { + trigger: 'focus', + html: true, + content: 'Action' + }) + + popoverEl.addEventListener('shown.bs.popover', () => { + const tip = document.querySelector('.popover') + const link = tip.querySelector('.inside-link') + + popoverEl.dispatchEvent(new FocusEvent('focusout', { + bubbles: true, + relatedTarget: link + })) + + expect(popover._activeTrigger.focus).toBeTrue() + expect(tip).toHaveClass('show') + resolve() + }) + + popoverEl.dispatchEvent(createEvent('focusin')) + }) + }) + + it('should hide when focus leaves focusable tip content for outside content', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = 'Dismissible' + + const popoverEl = fixtureEl.querySelector('a') + // eslint-disable-next-line no-new + new Popover(popoverEl, { + trigger: 'focus', + html: true, + content: 'Action' + }) + + popoverEl.addEventListener('shown.bs.popover', () => { + const tip = document.querySelector('.popover') + + popoverEl.addEventListener('hidden.bs.popover', () => { + expect(document.querySelector('.popover')).toBeNull() + resolve() + }) + + tip.dispatchEvent(new FocusEvent('focusout', { + bubbles: true, + relatedTarget: document.body + })) + }) + + popoverEl.dispatchEvent(createEvent('focusin')) + }) + }) + }) + describe('data-api', () => { it('should toggle popover on click via data-api', () => { return new Promise(resolve => { diff --git a/site/src/content/docs/components/popover.mdx b/site/src/content/docs/components/popover.mdx index 47c6c32e0ba5..c6c5f340dec7 100644 --- a/site/src/content/docs/components/popover.mdx +++ b/site/src/content/docs/components/popover.mdx @@ -14,7 +14,7 @@ deps: ## Overview -Click the button below to trigger a popover: +Click the button below to show and close a popover. **Dismissing on next click requires specific HTML for proper cross-browser and cross-platform behavior.** You can only use `` elements, not `