From 6ecd1c626e4129daf45a47b44c2e2eae60a09fa3 Mon Sep 17 00:00:00 2001 From: Rohit Sharma Date: Tue, 16 Mar 2021 10:51:04 +0530 Subject: [PATCH 1/6] Change the name of the `Offcanvas` constructor (#33261) --- build/build-plugins.js | 4 +-- js/index.esm.js | 4 +-- js/index.umd.js | 4 +-- js/src/offcanvas.js | 10 +++--- js/tests/unit/offcanvas.spec.js | 58 ++++++++++++++++----------------- 5 files changed, 40 insertions(+), 40 deletions(-) diff --git a/build/build-plugins.js b/build/build-plugins.js index 7175df4dd562..7fd58bcb647f 100644 --- a/build/build-plugins.js +++ b/build/build-plugins.js @@ -35,7 +35,7 @@ const bsPlugins = { Collapse: path.resolve(__dirname, '../js/src/collapse.js'), Dropdown: path.resolve(__dirname, '../js/src/dropdown.js'), Modal: path.resolve(__dirname, '../js/src/modal.js'), - OffCanvas: path.resolve(__dirname, '../js/src/offcanvas.js'), + Offcanvas: path.resolve(__dirname, '../js/src/offcanvas.js'), Popover: path.resolve(__dirname, '../js/src/popover.js'), ScrollSpy: path.resolve(__dirname, '../js/src/scrollspy.js'), Tab: path.resolve(__dirname, '../js/src/tab.js'), @@ -72,7 +72,7 @@ const getConfigByPluginKey = pluginKey => { } } - if (pluginKey === 'Alert' || pluginKey === 'Tab' || pluginKey === 'OffCanvas') { + if (pluginKey === 'Alert' || pluginKey === 'Tab' || pluginKey === 'Offcanvas') { return defaultPluginConfig } diff --git a/js/index.esm.js b/js/index.esm.js index c041beffe255..b95e63fabc11 100644 --- a/js/index.esm.js +++ b/js/index.esm.js @@ -11,7 +11,7 @@ import Carousel from './src/carousel' import Collapse from './src/collapse' import Dropdown from './src/dropdown' import Modal from './src/modal' -import OffCanvas from './src/offcanvas' +import Offcanvas from './src/offcanvas' import Popover from './src/popover' import ScrollSpy from './src/scrollspy' import Tab from './src/tab' @@ -25,7 +25,7 @@ export { Collapse, Dropdown, Modal, - OffCanvas, + Offcanvas, Popover, ScrollSpy, Tab, diff --git a/js/index.umd.js b/js/index.umd.js index a99a3be896a3..8e52cca0f191 100644 --- a/js/index.umd.js +++ b/js/index.umd.js @@ -11,7 +11,7 @@ import Carousel from './src/carousel' import Collapse from './src/collapse' import Dropdown from './src/dropdown' import Modal from './src/modal' -import OffCanvas from './src/offcanvas' +import Offcanvas from './src/offcanvas' import Popover from './src/popover' import ScrollSpy from './src/scrollspy' import Tab from './src/tab' @@ -25,7 +25,7 @@ export default { Collapse, Dropdown, Modal, - OffCanvas, + Offcanvas, Popover, ScrollSpy, Tab, diff --git a/js/src/offcanvas.js b/js/src/offcanvas.js index 148f003e9634..f4927aacd68f 100644 --- a/js/src/offcanvas.js +++ b/js/src/offcanvas.js @@ -54,7 +54,7 @@ const SELECTOR_DATA_TOGGLE = '[data-bs-toggle="offcanvas"]' * ------------------------------------------------------------------------ */ -class OffCanvas extends BaseComponent { +class Offcanvas extends BaseComponent { constructor(element) { super(element) @@ -181,7 +181,7 @@ class OffCanvas extends BaseComponent { static jQueryInterface(config) { return this.each(function () { - const data = Data.get(this, DATA_KEY) || new OffCanvas(this) + const data = Data.get(this, DATA_KEY) || new Offcanvas(this) if (typeof config === 'string') { if (typeof data[config] === 'undefined') { @@ -224,7 +224,7 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function ( return } - const data = Data.get(target, DATA_KEY) || new OffCanvas(target) + const data = Data.get(target, DATA_KEY) || new Offcanvas(target) data.toggle(this) }) @@ -234,6 +234,6 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function ( * ------------------------------------------------------------------------ */ -defineJQueryPlugin(NAME, OffCanvas) +defineJQueryPlugin(NAME, Offcanvas) -export default OffCanvas +export default Offcanvas diff --git a/js/tests/unit/offcanvas.spec.js b/js/tests/unit/offcanvas.spec.js index d07c0c61021f..07a7cf682b33 100644 --- a/js/tests/unit/offcanvas.spec.js +++ b/js/tests/unit/offcanvas.spec.js @@ -1,10 +1,10 @@ -import OffCanvas from '../../src/offcanvas' +import Offcanvas from '../../src/offcanvas' import EventHandler from '../../src/dom/event-handler' /** Test helpers */ import { clearFixture, getFixture, jQueryMock, createEvent } from '../helpers/fixture' -describe('OffCanvas', () => { +describe('Offcanvas', () => { let fixtureEl beforeAll(() => { @@ -18,7 +18,7 @@ describe('OffCanvas', () => { describe('VERSION', () => { it('should return plugin version', () => { - expect(OffCanvas.VERSION).toEqual(jasmine.any(String)) + expect(Offcanvas.VERSION).toEqual(jasmine.any(String)) }) }) @@ -32,7 +32,7 @@ describe('OffCanvas', () => { const offCanvasEl = fixtureEl.querySelector('.offcanvas') const closeEl = fixtureEl.querySelector('a') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) spyOn(offCanvas, 'hide') @@ -45,7 +45,7 @@ describe('OffCanvas', () => { fixtureEl.innerHTML = '
' const offCanvasEl = fixtureEl.querySelector('.offcanvas') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) const keyDownEsc = createEvent('keydown') keyDownEsc.key = 'Escape' @@ -60,7 +60,7 @@ describe('OffCanvas', () => { fixtureEl.innerHTML = '
' const offCanvasEl = fixtureEl.querySelector('.offcanvas') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) const keydownTab = createEvent('keydown') keydownTab.key = 'Tab' @@ -77,7 +77,7 @@ describe('OffCanvas', () => { fixtureEl.innerHTML = '
' const offCanvasEl = fixtureEl.querySelector('.offcanvas') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) spyOn(offCanvas, 'show') @@ -90,7 +90,7 @@ describe('OffCanvas', () => { fixtureEl.innerHTML = '
' const offCanvasEl = fixtureEl.querySelector('.show') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) spyOn(offCanvas, 'hide') @@ -107,7 +107,7 @@ describe('OffCanvas', () => { spyOn(EventHandler, 'trigger') const offCanvasEl = fixtureEl.querySelector('div') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) offCanvas.show() @@ -118,7 +118,7 @@ describe('OffCanvas', () => { fixtureEl.innerHTML = '
' const offCanvasEl = fixtureEl.querySelector('div') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) offCanvasEl.addEventListener('shown.bs.offcanvas', () => { expect(offCanvasEl.classList.contains('show')).toEqual(true) @@ -132,7 +132,7 @@ describe('OffCanvas', () => { fixtureEl.innerHTML = '
' const offCanvasEl = fixtureEl.querySelector('div') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) const expectEnd = () => { setTimeout(() => { @@ -161,7 +161,7 @@ describe('OffCanvas', () => { spyOn(EventHandler, 'trigger') const offCanvasEl = fixtureEl.querySelector('div') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) offCanvas.hide() @@ -172,7 +172,7 @@ describe('OffCanvas', () => { fixtureEl.innerHTML = '
' const offCanvasEl = fixtureEl.querySelector('div') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) offCanvasEl.addEventListener('hidden.bs.offcanvas', () => { expect(offCanvasEl.classList.contains('show')).toEqual(false) @@ -186,7 +186,7 @@ describe('OffCanvas', () => { fixtureEl.innerHTML = '
' const offCanvasEl = fixtureEl.querySelector('div') - const offCanvas = new OffCanvas(offCanvasEl) + const offCanvas = new Offcanvas(offCanvasEl) const expectEnd = () => { setTimeout(() => { @@ -235,11 +235,11 @@ describe('OffCanvas', () => { const target = fixtureEl.querySelector('a') - spyOn(OffCanvas.prototype, 'toggle') + spyOn(Offcanvas.prototype, 'toggle') target.click() - expect(OffCanvas.prototype.toggle).not.toHaveBeenCalled() + expect(Offcanvas.prototype.toggle).not.toHaveBeenCalled() }) }) @@ -249,26 +249,26 @@ describe('OffCanvas', () => { const div = fixtureEl.querySelector('div') - jQueryMock.fn.offcanvas = OffCanvas.jQueryInterface + jQueryMock.fn.offcanvas = Offcanvas.jQueryInterface jQueryMock.elements = [div] jQueryMock.fn.offcanvas.call(jQueryMock) - expect(OffCanvas.getInstance(div)).toBeDefined() + expect(Offcanvas.getInstance(div)).toBeDefined() }) it('should not re create an offcanvas', () => { fixtureEl.innerHTML = '
' const div = fixtureEl.querySelector('div') - const offCanvas = new OffCanvas(div) + const offCanvas = new Offcanvas(div) - jQueryMock.fn.offcanvas = OffCanvas.jQueryInterface + jQueryMock.fn.offcanvas = Offcanvas.jQueryInterface jQueryMock.elements = [div] jQueryMock.fn.offcanvas.call(jQueryMock) - expect(OffCanvas.getInstance(div)).toEqual(offCanvas) + expect(Offcanvas.getInstance(div)).toEqual(offCanvas) }) it('should throw error on undefined method', () => { @@ -277,7 +277,7 @@ describe('OffCanvas', () => { const div = fixtureEl.querySelector('div') const action = 'undefinedMethod' - jQueryMock.fn.offcanvas = OffCanvas.jQueryInterface + jQueryMock.fn.offcanvas = Offcanvas.jQueryInterface jQueryMock.elements = [div] try { @@ -292,13 +292,13 @@ describe('OffCanvas', () => { const div = fixtureEl.querySelector('div') - spyOn(OffCanvas.prototype, 'show') + spyOn(Offcanvas.prototype, 'show') - jQueryMock.fn.offcanvas = OffCanvas.jQueryInterface + jQueryMock.fn.offcanvas = Offcanvas.jQueryInterface jQueryMock.elements = [div] jQueryMock.fn.offcanvas.call(jQueryMock, 'show') - expect(OffCanvas.prototype.show).toHaveBeenCalled() + expect(Offcanvas.prototype.show).toHaveBeenCalled() }) }) @@ -307,10 +307,10 @@ describe('OffCanvas', () => { fixtureEl.innerHTML = '
' const div = fixtureEl.querySelector('div') - const offCanvas = new OffCanvas(div) + const offCanvas = new Offcanvas(div) - expect(OffCanvas.getInstance(div)).toEqual(offCanvas) - expect(OffCanvas.getInstance(div)).toBeInstanceOf(OffCanvas) + expect(Offcanvas.getInstance(div)).toEqual(offCanvas) + expect(Offcanvas.getInstance(div)).toBeInstanceOf(Offcanvas) }) it('should return null when there is no offcanvas instance', () => { @@ -318,7 +318,7 @@ describe('OffCanvas', () => { const div = fixtureEl.querySelector('div') - expect(OffCanvas.getInstance(div)).toEqual(null) + expect(Offcanvas.getInstance(div)).toEqual(null) }) }) }) From d491c29aa005177ef148c40d4b7b0a3decc7edef Mon Sep 17 00:00:00 2001 From: Ryan Berliner Date: Sat, 6 Mar 2021 03:35:28 +0200 Subject: [PATCH 2/6] prevent tooltip from being deleted on quick re-activations --- js/src/tooltip.js | 4 ++++ js/tests/unit/tooltip.spec.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/js/src/tooltip.js b/js/src/tooltip.js index 6f33245f8ee0..e9f9bfff1b44 100644 --- a/js/src/tooltip.js +++ b/js/src/tooltip.js @@ -329,6 +329,10 @@ class Tooltip extends BaseComponent { const tip = this.getTipElement() const complete = () => { + if (this._isWithActiveTrigger()) { + return + } + if (this._hoverState !== HOVER_STATE_SHOW && tip.parentNode) { tip.parentNode.removeChild(tip) } diff --git a/js/tests/unit/tooltip.spec.js b/js/tests/unit/tooltip.spec.js index 7bf6aa3ab837..41f73d6d8a7d 100644 --- a/js/tests/unit/tooltip.spec.js +++ b/js/tests/unit/tooltip.spec.js @@ -708,6 +708,35 @@ describe('Tooltip', () => { tooltipEl.dispatchEvent(createEvent('mouseover')) }) + it('should not hide tooltip if leave event occurs and enter event occurs during hide transition', done => { + fixtureEl.innerHTML = '' + + const tooltipEl = fixtureEl.querySelector('a') + const tooltip = new Tooltip(tooltipEl) + + spyOn(window, 'getComputedStyle').and.returnValue({ + transitionDuration: '0.15s', + transitionDelay: '0s' + }) + + setTimeout(() => { + expect(tooltip._popper).not.toBeNull() + tooltipEl.dispatchEvent(createEvent('mouseout')) + + setTimeout(() => { + expect(tooltip.getTipElement().classList.contains('show')).toEqual(false) + tooltipEl.dispatchEvent(createEvent('mouseover')) + }, 100) + + setTimeout(() => { + expect(tooltip._popper).not.toBeNull() + done() + }, 200) + }, 0) + + tooltipEl.dispatchEvent(createEvent('mouseover')) + }) + it('should show a tooltip with custom class provided in data attributes', done => { fixtureEl.innerHTML = '' From 6ef70b342c27445685715f51cfcafb719356870f Mon Sep 17 00:00:00 2001 From: Ryan Berliner Date: Sat, 6 Mar 2021 23:57:23 +0200 Subject: [PATCH 3/6] prevent quick interactions from misplacing tooltips --- js/src/tooltip.js | 6 +++++- js/tests/unit/tooltip.spec.js | 7 +++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/js/src/tooltip.js b/js/src/tooltip.js index e9f9bfff1b44..857f72c8ad2c 100644 --- a/js/src/tooltip.js +++ b/js/src/tooltip.js @@ -283,6 +283,10 @@ class Tooltip extends BaseComponent { EventHandler.trigger(this._element, this.constructor.Event.INSERTED) + if (this._popper) { + this._popper.destroy() + } + this._popper = Popper.createPopper(this._element, tip, this._getPopperConfig(attachment)) tip.classList.add(CLASS_NAME_SHOW) @@ -650,7 +654,7 @@ class Tooltip extends BaseComponent { if (event) { context._activeTrigger[ event.type === 'focusout' ? TRIGGER_FOCUS : TRIGGER_HOVER - ] = false + ] = context._element.contains(event.relatedTarget) } if (context._isWithActiveTrigger()) { diff --git a/js/tests/unit/tooltip.spec.js b/js/tests/unit/tooltip.spec.js index 41f73d6d8a7d..1cb301c151aa 100644 --- a/js/tests/unit/tooltip.spec.js +++ b/js/tests/unit/tooltip.spec.js @@ -708,8 +708,9 @@ describe('Tooltip', () => { tooltipEl.dispatchEvent(createEvent('mouseover')) }) - it('should not hide tooltip if leave event occurs and enter event occurs during hide transition', done => { - fixtureEl.innerHTML = '' + it('should properly maintain tooltip state if leave event occurs and enter event occurs during hide transition', done => { + // Style this tooltip to give it plenty of room for popper to do what it wants + fixtureEl.innerHTML = 'Trigger' const tooltipEl = fixtureEl.querySelector('a') const tooltip = new Tooltip(tooltipEl) @@ -721,6 +722,7 @@ describe('Tooltip', () => { setTimeout(() => { expect(tooltip._popper).not.toBeNull() + expect(tooltip.getTipElement().getAttribute('data-popper-placement')).toBe('top') tooltipEl.dispatchEvent(createEvent('mouseout')) setTimeout(() => { @@ -730,6 +732,7 @@ describe('Tooltip', () => { setTimeout(() => { expect(tooltip._popper).not.toBeNull() + expect(tooltip.getTipElement().getAttribute('data-popper-placement')).toBe('top') done() }, 200) }, 0) From 72d23135799059d4282ea5764455f92f39ced5a5 Mon Sep 17 00:00:00 2001 From: Ryan Berliner Date: Sun, 7 Mar 2021 08:28:41 -0500 Subject: [PATCH 4/6] reuse existing popper on show during tooltip fadeout --- js/src/tooltip.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/src/tooltip.js b/js/src/tooltip.js index 857f72c8ad2c..de7dcca69391 100644 --- a/js/src/tooltip.js +++ b/js/src/tooltip.js @@ -284,11 +284,11 @@ class Tooltip extends BaseComponent { EventHandler.trigger(this._element, this.constructor.Event.INSERTED) if (this._popper) { - this._popper.destroy() + this._popper.update() + } else { + this._popper = Popper.createPopper(this._element, tip, this._getPopperConfig(attachment)) } - this._popper = Popper.createPopper(this._element, tip, this._getPopperConfig(attachment)) - tip.classList.add(CLASS_NAME_SHOW) const customClass = typeof this.config.customClass === 'function' ? this.config.customClass() : this.config.customClass From 99b2c0b390660b2032c3129b8ebff02fa1e034c9 Mon Sep 17 00:00:00 2001 From: Ryan Berliner Date: Sun, 7 Mar 2021 17:09:17 +0200 Subject: [PATCH 5/6] only trigger tooltip inserted event on true dom insert --- js/src/tooltip.js | 3 +- js/tests/unit/tooltip.spec.js | 62 +++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/js/src/tooltip.js b/js/src/tooltip.js index de7dcca69391..979bd077382c 100644 --- a/js/src/tooltip.js +++ b/js/src/tooltip.js @@ -279,10 +279,9 @@ class Tooltip extends BaseComponent { if (!this._element.ownerDocument.documentElement.contains(this.tip)) { container.appendChild(tip) + EventHandler.trigger(this._element, this.constructor.Event.INSERTED) } - EventHandler.trigger(this._element, this.constructor.Event.INSERTED) - if (this._popper) { this._popper.update() } else { diff --git a/js/tests/unit/tooltip.spec.js b/js/tests/unit/tooltip.spec.js index 1cb301c151aa..f9d97e3f7ed4 100644 --- a/js/tests/unit/tooltip.spec.js +++ b/js/tests/unit/tooltip.spec.js @@ -708,6 +708,37 @@ describe('Tooltip', () => { tooltipEl.dispatchEvent(createEvent('mouseover')) }) + it('should not hide tooltip if leave event occurs and interaction remains inside trigger', done => { + fixtureEl.innerHTML = [ + '', + 'Trigger', + 'the tooltip', + '' + ] + + const tooltipEl = fixtureEl.querySelector('a') + const tooltip = new Tooltip(tooltipEl) + const triggerChild = tooltipEl.querySelector('b') + + spyOn(tooltip, 'hide').and.callThrough() + + tooltipEl.addEventListener('mouseover', () => { + const moveMouseToChildEvent = createEvent('mouseout') + Object.defineProperty(moveMouseToChildEvent, 'relatedTarget', { + value: triggerChild + }) + + tooltipEl.dispatchEvent(moveMouseToChildEvent) + }) + + tooltipEl.addEventListener('mouseout', () => { + expect(tooltip.hide).not.toHaveBeenCalled() + done() + }) + + tooltipEl.dispatchEvent(createEvent('mouseover')) + }) + it('should properly maintain tooltip state if leave event occurs and enter event occurs during hide transition', done => { // Style this tooltip to give it plenty of room for popper to do what it wants fixtureEl.innerHTML = 'Trigger' @@ -740,6 +771,37 @@ describe('Tooltip', () => { tooltipEl.dispatchEvent(createEvent('mouseover')) }) + it('should only trigger inserted event if a new tooltip element was created', done => { + fixtureEl.innerHTML = '' + + const tooltipEl = fixtureEl.querySelector('a') + const tooltip = new Tooltip(tooltipEl) + + spyOn(window, 'getComputedStyle').and.returnValue({ + transitionDuration: '0.15s', + transitionDelay: '0s' + }) + + const insertedFunc = jasmine.createSpy() + tooltipEl.addEventListener('inserted.bs.tooltip', insertedFunc) + + setTimeout(() => { + expect(insertedFunc).toHaveBeenCalledTimes(1) + tooltip.hide() + + setTimeout(() => { + tooltip.show() + }, 100) + + setTimeout(() => { + expect(insertedFunc).toHaveBeenCalledTimes(1) + done() + }, 200) + }, 0) + + tooltip.show() + }) + it('should show a tooltip with custom class provided in data attributes', done => { fixtureEl.innerHTML = '' From af42557f4a8eb69bd098869d0f29f4c456c95063 Mon Sep 17 00:00:00 2001 From: Anton Date: Tue, 16 Mar 2021 08:49:52 +0300 Subject: [PATCH 6/6] Update modal's `show` method to accept `relatedTarget` as an argument (#33300) Co-authored-by: Rohit Sharma Co-authored-by: XhmikosR --- site/content/docs/5.0/components/modal.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/site/content/docs/5.0/components/modal.md b/site/content/docs/5.0/components/modal.md index 0abe0ce0f2fa..0f11614fdeaf 100644 --- a/site/content/docs/5.0/components/modal.md +++ b/site/content/docs/5.0/components/modal.md @@ -892,6 +892,13 @@ Manually opens a modal. **Returns to the caller before the modal has actually be myModal.show() ``` +Also, you can pass a DOM element as an argument that can be received in the modal events (as the `relatedTarget` property). + +```js +var modalToggle = document.getElementById('toggleMyModal') // relatedTarget +myModal.show(modalToggle) +``` + #### hide Manually hides a modal. **Returns to the caller before the modal has actually been hidden** (i.e. before the `hidden.bs.modal` event occurs).